%s
' % msg.replace('<', '<') msg += '%s' % traceback.format_exc().replace('<', '<') # create a dummy problem with error message instead of failing @@ -152,7 +161,8 @@ class CapaModule(XModule): problem_text, self.location.html_id(), instance_state, seed=seed, system=self.system) else: - raise + # add extra info and raise + raise Exception(msg), None, sys.exc_info()[2] @property def rerandomize(self): @@ -191,6 +201,7 @@ class CapaModule(XModule): try: return Progress(score, total) except Exception as err: + # TODO (vshnayder): why is this still here? still needed? if self.system.DEBUG: return None raise @@ -210,6 +221,7 @@ class CapaModule(XModule): try: html = self.lcp.get_html() except Exception, err: + # TODO (vshnayder): another switch on DEBUG. if self.system.DEBUG: log.exception(err) msg = ( @@ -561,6 +573,7 @@ class CapaDescriptor(RawDescriptor): 'problems/' + path[8:], path[8:], ] + @classmethod def split_to_file(cls, xml_object): '''Problems always written in their own files''' diff --git a/common/lib/xmodule/xmodule/course_module.py b/common/lib/xmodule/xmodule/course_module.py index 7e0019205e..acdc574220 100644 --- a/common/lib/xmodule/xmodule/course_module.py +++ b/common/lib/xmodule/xmodule/course_module.py @@ -20,15 +20,22 @@ class CourseDescriptor(SequenceDescriptor): self._grader = None self._grade_cutoffs = None + msg = None try: self.start = time.strptime(self.metadata["start"], "%Y-%m-%dT%H:%M") except KeyError: self.start = time.gmtime(0) #The epoch - log.critical("Course loaded without a start date. %s", self.id) + msg = "Course loaded without a start date. id = %s" % self.id + log.critical(msg) except ValueError as e: self.start = time.gmtime(0) #The epoch - log.critical("Course loaded with a bad start date. %s '%s'", - self.id, e) + msg = "Course loaded with a bad start date. %s '%s'" % (self.id, e) + log.critical(msg) + + # Don't call the tracker from the exception handler. + if msg is not None: + system.error_tracker(msg) + def has_started(self): return time.gmtime() > self.start @@ -104,3 +111,4 @@ class CourseDescriptor(SequenceDescriptor): @property def org(self): return self.location.org + diff --git a/common/lib/xmodule/xmodule/editing_module.py b/common/lib/xmodule/xmodule/editing_module.py new file mode 100644 index 0000000000..4188165a24 --- /dev/null +++ b/common/lib/xmodule/xmodule/editing_module.py @@ -0,0 +1,24 @@ +from pkg_resources import resource_string +from lxml import etree +from xmodule.mako_module import MakoModuleDescriptor +import logging + +log = logging.getLogger(__name__) + +class EditingDescriptor(MakoModuleDescriptor): + """ + Module that provides a raw editing view of its data and children. It does not + perform any validation on its definition---just passes it along to the browser. + + This class is intended to be used as a mixin. + """ + mako_template = "widgets/raw-edit.html" + + js = {'coffee': [resource_string(__name__, 'js/src/raw/edit.coffee')]} + js_module_name = "RawDescriptor" + + def get_context(self): + return { + 'module': self, + 'data': self.definition['data'], + } diff --git a/common/lib/xmodule/xmodule/error_module.py b/common/lib/xmodule/xmodule/error_module.py new file mode 100644 index 0000000000..62b3b82a39 --- /dev/null +++ b/common/lib/xmodule/xmodule/error_module.py @@ -0,0 +1,80 @@ +from pkg_resources import resource_string +from lxml import etree +from xmodule.x_module import XModule +from xmodule.mako_module import MakoModuleDescriptor +from xmodule.xml_module import XmlDescriptor +from xmodule.editing_module import EditingDescriptor + +import logging + +log = logging.getLogger(__name__) + +class ErrorModule(XModule): + def get_html(self): + '''Show an error. + TODO (vshnayder): proper style, divs, etc. + ''' + if not self.system.is_staff: + return self.system.render_template('module-error.html', {}) + + # staff get to see all the details + return self.system.render_template('module-error-staff.html', { + 'data' : self.definition['data'], + # TODO (vshnayder): need to get non-syntax errors in here somehow + 'error' : self.definition.get('error', 'Error not available') + }) + +class ErrorDescriptor(EditingDescriptor): + """ + Module that provides a raw editing view of broken xml. + """ + module_class = ErrorModule + + @classmethod + def from_xml(cls, xml_data, system, org=None, course=None, err=None): + '''Create an instance of this descriptor from the supplied data. + + Does not try to parse the data--just stores it. + + Takes an extra, optional, parameter--the error that caused an + issue. + ''' + + definition = {} + if err is not None: + definition['error'] = err + + try: + # If this is already an error tag, don't want to re-wrap it. + xml_obj = etree.fromstring(xml_data) + if xml_obj.tag == 'error': + xml_data = xml_obj.text + except etree.XMLSyntaxError as err: + # Save the error to display later--overrides other problems + definition['error'] = err + + definition['data'] = xml_data + # TODO (vshnayder): Do we need a unique slug here? Just pick a random + # 64-bit num? + location = ['i4x', org, course, 'error', 'slug'] + metadata = {} # stays in the xml_data + + return cls(system, definition, location=location, metadata=metadata) + + def export_to_xml(self, resource_fs): + ''' + If the definition data is invalid xml, export it wrapped in an "error" + tag. If it is valid, export without the wrapper. + + NOTE: There may still be problems with the valid xml--it could be + missing required attributes, could have the wrong tags, refer to missing + files, etc. That would just get re-wrapped on import. + ''' + try: + xml = etree.fromstring(self.definition['data']) + return etree.tostring(xml) + except etree.XMLSyntaxError: + # still not valid. + root = etree.Element('error') + root.text = self.definition['data'] + return etree.tostring(root) diff --git a/common/lib/xmodule/xmodule/errorhandlers.py b/common/lib/xmodule/xmodule/errorhandlers.py deleted file mode 100644 index 0f97377b2a..0000000000 --- a/common/lib/xmodule/xmodule/errorhandlers.py +++ /dev/null @@ -1,45 +0,0 @@ -import logging -import sys - -log = logging.getLogger(__name__) - -def in_exception_handler(): - '''Is there an active exception?''' - return sys.exc_info() != (None, None, None) - -def strict_error_handler(msg, exc_info=None): - ''' - Do not let errors pass. If exc_info is not None, ignore msg, and just - re-raise. Otherwise, check if we are in an exception-handling context. - If so, re-raise. Otherwise, raise Exception(msg). - - Meant for use in validation, where any errors should trap. - ''' - if exc_info is not None: - raise exc_info[0], exc_info[1], exc_info[2] - - if in_exception_handler(): - raise - - raise Exception(msg) - - -def logging_error_handler(msg, exc_info=None): - '''Log all errors, but otherwise let them pass, relying on the caller to - workaround.''' - if exc_info is not None: - log.exception(msg, exc_info=exc_info) - return - - if in_exception_handler(): - log.exception(msg) - return - - log.error(msg) - - -def ignore_errors_handler(msg, exc_info=None): - '''Ignore all errors, relying on the caller to workaround. - Meant for use in the LMS, where an error in one part of the course - shouldn't bring down the whole system''' - pass diff --git a/common/lib/xmodule/xmodule/errortracker.py b/common/lib/xmodule/xmodule/errortracker.py new file mode 100644 index 0000000000..b8d42a6983 --- /dev/null +++ b/common/lib/xmodule/xmodule/errortracker.py @@ -0,0 +1,38 @@ +import logging +import sys +import traceback + +from collections import namedtuple + +log = logging.getLogger(__name__) + +ErrorLog = namedtuple('ErrorLog', 'tracker errors') + +def in_exception_handler(): + '''Is there an active exception?''' + return sys.exc_info() != (None, None, None) + + +def make_error_tracker(): + '''Return an ErrorLog (named tuple), with fields (tracker, errors), where + the logger appends a tuple (message, exception_str) to the errors on every + call. exception_str is in the format returned by traceback.format_exception. + + error_list is a simple list. If the caller modifies it, info + will be lost. + ''' + errors = [] + + def error_tracker(msg): + '''Log errors''' + exc_str = '' + if in_exception_handler(): + exc_str = ''.join(traceback.format_exception(*sys.exc_info())) + + errors.append((msg, exc_str)) + + return ErrorLog(error_tracker, errors) + +def null_error_tracker(msg): + '''A dummy error tracker that just ignores the messages''' + pass diff --git a/common/lib/xmodule/xmodule/html_module.py b/common/lib/xmodule/xmodule/html_module.py index b9bc34aed6..b2a5df9803 100644 --- a/common/lib/xmodule/xmodule/html_module.py +++ b/common/lib/xmodule/xmodule/html_module.py @@ -1,13 +1,18 @@ +import copy +from fs.errors import ResourceNotFoundError import logging import os +import sys from lxml import etree from xmodule.x_module import XModule -from xmodule.raw_module import RawDescriptor +from xmodule.xml_module import XmlDescriptor +from xmodule.editing_module import EditingDescriptor +from stringify import stringify_children +from html_checker import check_html log = logging.getLogger("mitx.courseware") - class HtmlModule(XModule): def get_html(self): return self.html @@ -19,33 +24,110 @@ class HtmlModule(XModule): self.html = self.definition['data'] -class HtmlDescriptor(RawDescriptor): +class HtmlDescriptor(XmlDescriptor, EditingDescriptor): """ Module for putting raw html in a course """ mako_template = "widgets/html-edit.html" module_class = HtmlModule - filename_extension = "html" + filename_extension = "xml" - # TODO (cpennington): Delete this method once all fall 2012 course are being - # edited in the cms + # VS[compat] TODO (cpennington): Delete this method once all fall 2012 course + # are being edited in the cms @classmethod def backcompat_paths(cls, path): - if path.endswith('.html.html'): - path = path[:-5] + origpath = path + if path.endswith('.html.xml'): + path = path[:-9] + '.html' #backcompat--look for html instead of xml candidates = [] while os.sep in path: candidates.append(path) _, _, path = path.partition(os.sep) + # also look for .html versions instead of .xml + if origpath.endswith('.xml'): + candidates.append(origpath[:-4] + '.html') return candidates + # NOTE: html descriptors are special. We do not want to parse and + # export them ourselves, because that can break things (e.g. lxml + # adds body tags when it exports, but they should just be html + # snippets that will be included in the middle of pages. + @classmethod - def file_to_xml(cls, file_object): - parser = etree.HTMLParser() - return etree.parse(file_object, parser).getroot() + def load_definition(cls, xml_object, system, location): + '''Load a descriptor from the specified xml_object: + + If there is a filename attribute, load it as a string, and + log a warning if it is not parseable by etree.HTMLParser. + + If there is not a filename attribute, the definition is the body + of the xml_object, without the root tag (do not want in the + middle of a page) + ''' + filename = xml_object.get('filename') + if filename is None: + definition_xml = copy.deepcopy(xml_object) + cls.clean_metadata_from_xml(definition_xml) + return {'data' : stringify_children(definition_xml)} + else: + filepath = cls._format_filepath(xml_object.tag, filename) + + # VS[compat] + # TODO (cpennington): If the file doesn't exist at the right path, + # give the class a chance to fix it up. The file will be written out + # again in the correct format. This should go away once the CMS is + # online and has imported all current (fall 2012) courses from xml + if not system.resources_fs.exists(filepath): + candidates = cls.backcompat_paths(filepath) + #log.debug("candidates = {0}".format(candidates)) + for candidate in candidates: + if system.resources_fs.exists(candidate): + filepath = candidate + break + + try: + with system.resources_fs.open(filepath) as file: + html = file.read() + # Log a warning if we can't parse the file, but don't error + if not check_html(html): + msg = "Couldn't parse html in {0}.".format(filepath) + log.warning(msg) + system.error_tracker("Warning: " + msg) + return {'data' : html} + except (ResourceNotFoundError) as err: + msg = 'Unable to load file contents at path {0}: {1} '.format( + filepath, err) + # add more info and re-raise + raise Exception(msg), None, sys.exc_info()[2] @classmethod def split_to_file(cls, xml_object): - # never include inline html + '''Never include inline html''' return True + + + # TODO (vshnayder): make export put things in the right places. + + def definition_to_xml(self, resource_fs): + '''If the contents are valid xml, write them to filename.xml. Otherwise, + write just the tag to filename.xml, and the html + string to filename.html. + ''' + try: + return etree.fromstring(self.definition['data']) + except etree.XMLSyntaxError: + pass + + # Not proper format. Write html to file, return an empty tag + filepath = u'{category}/{name}.html'.format(category=self.category, + name=self.name) + + resource_fs.makedir(os.path.dirname(filepath), allow_recreate=True) + with resource_fs.open(filepath, 'w') as file: + file.write(self.definition['data']) + + elt = etree.Element('html') + elt.set("filename", self.name) + return elt + diff --git a/common/lib/xmodule/xmodule/mako_module.py b/common/lib/xmodule/xmodule/mako_module.py index fcc47aaaaf..eedac99aa8 100644 --- a/common/lib/xmodule/xmodule/mako_module.py +++ b/common/lib/xmodule/xmodule/mako_module.py @@ -2,10 +2,10 @@ from x_module import XModuleDescriptor, DescriptorSystem class MakoDescriptorSystem(DescriptorSystem): - def __init__(self, load_item, resources_fs, error_handler, - render_template): + def __init__(self, load_item, resources_fs, error_tracker, + render_template, **kwargs): super(MakoDescriptorSystem, self).__init__( - load_item, resources_fs, error_handler) + load_item, resources_fs, error_tracker, **kwargs) self.render_template = render_template diff --git a/common/lib/xmodule/xmodule/modulestore/__init__.py b/common/lib/xmodule/xmodule/modulestore/__init__.py index 279782b61a..7241179d8e 100644 --- a/common/lib/xmodule/xmodule/modulestore/__init__.py +++ b/common/lib/xmodule/xmodule/modulestore/__init__.py @@ -5,7 +5,7 @@ that are stored in a database an accessible using their Location as an identifie import re from collections import namedtuple -from .exceptions import InvalidLocationError +from .exceptions import InvalidLocationError, InsufficientSpecificationError import logging log = logging.getLogger('mitx.' + 'modulestore') @@ -38,15 +38,15 @@ class Location(_LocationBase): ''' __slots__ = () - @classmethod - def clean(cls, value): + @staticmethod + def clean(value): """ Return value, made into a form legal for locations """ return re.sub('_+', '_', INVALID_CHARS.sub('_', value)) - @classmethod - def is_valid(cls, value): + @staticmethod + def is_valid(value): ''' Check if the value is a valid location, in any acceptable format. ''' @@ -56,6 +56,21 @@ class Location(_LocationBase): return False return True + @staticmethod + def ensure_fully_specified(location): + '''Make sure location is valid, and fully specified. Raises + InvalidLocationError or InsufficientSpecificationError if not. + + returns a Location object corresponding to location. + ''' + loc = Location(location) + for key, val in loc.dict().iteritems(): + if key != 'revision' and val is None: + raise InsufficientSpecificationError(location) + return loc + + + def __new__(_cls, loc_or_tag=None, org=None, course=None, category=None, name=None, revision=None): """ @@ -198,6 +213,18 @@ class ModuleStore(object): """ raise NotImplementedError + def get_item_errors(self, location): + """ + Return a list of (msg, exception-or-None) errors that the modulestore + encountered when loading the item at location. + + location : something that can be passed to Location + + Raises the same exceptions as get_item if the location isn't found or + isn't fully specified. + """ + raise NotImplementedError + def get_items(self, location, depth=0): """ Returns a list of XModuleDescriptor instances for the items @@ -254,25 +281,12 @@ class ModuleStore(object): ''' raise NotImplementedError - def path_to_location(self, location, course=None, chapter=None, section=None): - ''' - Try to find a course/chapter/section[/position] path to this location. - raise ItemNotFoundError if the location doesn't exist. + def get_parent_locations(self, location): + '''Find all locations that are the parents of this location. Needed + for path_to_location(). - If course, chapter, section are not None, restrict search to paths with those - components as specified. - - raise NoPathToItem if the location exists, but isn't accessible via - a path that matches the course/chapter/section restrictions. - - In general, a location may be accessible via many paths. This method may - return any valid path. - - Return a tuple (course, chapter, section, position). - - If the section a sequence, position should be the position of this location - in that sequence. Otherwise, position should be None. + returns an iterable of things that can be passed to Location. ''' raise NotImplementedError diff --git a/common/lib/xmodule/xmodule/modulestore/mongo.py b/common/lib/xmodule/xmodule/modulestore/mongo.py index df4e20f3a7..76769b25b0 100644 --- a/common/lib/xmodule/xmodule/modulestore/mongo.py +++ b/common/lib/xmodule/xmodule/modulestore/mongo.py @@ -6,14 +6,13 @@ from itertools import repeat from path import path from importlib import import_module -from xmodule.errorhandlers import strict_error_handler +from xmodule.errortracker import null_error_tracker from xmodule.x_module import XModuleDescriptor from xmodule.mako_module import MakoDescriptorSystem -from xmodule.course_module import CourseDescriptor from mitxmako.shortcuts import render_to_string from . import ModuleStore, Location -from .exceptions import (ItemNotFoundError, InsufficientSpecificationError, +from .exceptions import (ItemNotFoundError, NoPathToItem, DuplicateItemError) # TODO (cpennington): This code currently operates under the assumption that @@ -27,7 +26,7 @@ class CachingDescriptorSystem(MakoDescriptorSystem): from, with a backup of calling to the underlying modulestore for more data """ def __init__(self, modulestore, module_data, default_class, resources_fs, - error_handler, render_template): + error_tracker, render_template): """ modulestore: the module store that can be used to retrieve additional modules @@ -39,13 +38,13 @@ class CachingDescriptorSystem(MakoDescriptorSystem): resources_fs: a filesystem, as per MakoDescriptorSystem - error_handler: + error_tracker: render_template: a function for rendering templates, as per MakoDescriptorSystem """ super(CachingDescriptorSystem, self).__init__( - self.load_item, resources_fs, error_handler, render_template) + self.load_item, resources_fs, error_tracker, render_template) self.modulestore = modulestore self.module_data = module_data self.default_class = default_class @@ -80,7 +79,8 @@ class MongoModuleStore(ModuleStore): """ # TODO (cpennington): Enable non-filesystem filestores - def __init__(self, host, db, collection, fs_root, port=27017, default_class=None): + def __init__(self, host, db, collection, fs_root, port=27017, default_class=None, + error_tracker=null_error_tracker): self.collection = pymongo.connection.Connection( host=host, port=port @@ -91,13 +91,17 @@ class MongoModuleStore(ModuleStore): # Force mongo to maintain an index over _id.* that is in the same order # that is used when querying by a location - self.collection.ensure_index(zip(('_id.' + field for field in Location._fields), repeat(1))) + self.collection.ensure_index( + zip(('_id.' + field for field in Location._fields), repeat(1))) - # TODO (vshnayder): default arg default_class=None will make this error - module_path, _, class_name = default_class.rpartition('.') - class_ = getattr(import_module(module_path), class_name) - self.default_class = class_ + if default_class is not None: + module_path, _, class_name = default_class.rpartition('.') + class_ = getattr(import_module(module_path), class_name) + self.default_class = class_ + else: + self.default_class = None self.fs_root = path(fs_root) + self.error_tracker = error_tracker def _clean_item_data(self, item): """ @@ -149,7 +153,7 @@ class MongoModuleStore(ModuleStore): data_cache, self.default_class, resource_fs, - strict_error_handler, + self.error_tracker, render_to_string, ) return system.load_item(item['location']) @@ -172,12 +176,17 @@ class MongoModuleStore(ModuleStore): return self.get_items(course_filter) def _find_one(self, location): - '''Look for a given location in the collection. - If revision isn't specified, returns the latest.''' - return self.collection.find_one( + '''Look for a given location in the collection. If revision is not + specified, returns the latest. If the item is not present, raise + ItemNotFoundError. + ''' + item = self.collection.find_one( location_to_query(location), sort=[('revision', pymongo.ASCENDING)], ) + if item is None: + raise ItemNotFoundError(location) + return item def get_item(self, location, depth=0): """ @@ -197,14 +206,8 @@ class MongoModuleStore(ModuleStore): calls to get_children() to cache. None indicates to cache all descendents. """ - - for key, val in Location(location).dict().iteritems(): - if key != 'revision' and val is None: - raise InsufficientSpecificationError(location) - + location = Location.ensure_fully_specified(location) item = self._find_one(location) - if item is None: - raise ItemNotFoundError(location) return self._load_items([item], depth)[0] def get_items(self, location, depth=0): @@ -282,96 +285,20 @@ class MongoModuleStore(ModuleStore): ) def get_parent_locations(self, location): - '''Find all locations that are the parents of this location. - Mostly intended for use in path_to_location, but exposed for testing - and possible other usefulness. + '''Find all locations that are the parents of this location. Needed + for path_to_location(). - returns an iterable of things that can be passed to Location. + If there is no data at location in this modulestore, raise + ItemNotFoundError. + + returns an iterable of things that can be passed to Location. This may + be empty if there are no parents. ''' - location = Location(location) - items = self.collection.find({'definition.children': str(location)}, + location = Location.ensure_fully_specified(location) + # Check that it's actually in this modulestore. + item = self._find_one(location) + # now get the parents + items = self.collection.find({'definition.children': location.url()}, {'_id': True}) return [i['_id'] for i in items] - def path_to_location(self, location, course_name=None): - ''' - Try to find a course_id/chapter/section[/position] path to this location. - The courseware insists that the first level in the course is chapter, - but any kind of module can be a "section". - - location: something that can be passed to Location - course_name: [optional]. If not None, restrict search to paths - in that course. - - raise ItemNotFoundError if the location doesn't exist. - - raise NoPathToItem if the location exists, but isn't accessible via - a chapter/section path in the course(s) being searched. - - Return a tuple (course_id, chapter, section, position) suitable for the - courseware index view. - - A location may be accessible via many paths. This method may - return any valid path. - - If the section is a sequence, position will be the position - of this location in that sequence. Otherwise, position will - be None. TODO (vshnayder): Not true yet. - ''' - # Check that location is present at all - if self._find_one(location) is None: - raise ItemNotFoundError(location) - - def flatten(xs): - '''Convert lisp-style (a, (b, (c, ()))) lists into a python list. - Not a general flatten function. ''' - p = [] - while xs != (): - p.append(xs[0]) - xs = xs[1] - return p - - def find_path_to_course(location, course_name=None): - '''Find a path up the location graph to a node with the - specified category. If no path exists, return None. If a - path exists, return it as a list with target location - first, and the starting location last. - ''' - # Standard DFS - - # To keep track of where we came from, the work queue has - # tuples (location, path-so-far). To avoid lots of - # copying, the path-so-far is stored as a lisp-style - # list--nested hd::tl tuples, and flattened at the end. - queue = [(location, ())] - while len(queue) > 0: - (loc, path) = queue.pop() # Takes from the end - loc = Location(loc) - # print 'Processing loc={0}, path={1}'.format(loc, path) - if loc.category == "course": - if course_name is None or course_name == loc.name: - # Found it! - path = (loc, path) - return flatten(path) - - # otherwise, add parent locations at the end - newpath = (loc, path) - parents = self.get_parent_locations(loc) - queue.extend(zip(parents, repeat(newpath))) - - # If we're here, there is no path - return None - - path = find_path_to_course(location, course_name) - if path is None: - raise(NoPathToItem(location)) - - n = len(path) - course_id = CourseDescriptor.location_to_id(path[0]) - chapter = path[1].name if n > 1 else None - section = path[2].name if n > 2 else None - - # TODO (vshnayder): not handling position at all yet... - position = None - - return (course_id, chapter, section, position) diff --git a/common/lib/xmodule/xmodule/modulestore/search.py b/common/lib/xmodule/xmodule/modulestore/search.py new file mode 100644 index 0000000000..a383b3f8ec --- /dev/null +++ b/common/lib/xmodule/xmodule/modulestore/search.py @@ -0,0 +1,96 @@ +from itertools import repeat + +from xmodule.course_module import CourseDescriptor + +from .exceptions import (ItemNotFoundError, NoPathToItem) +from . import ModuleStore, Location + + +def path_to_location(modulestore, location, course_name=None): + ''' + Try to find a course_id/chapter/section[/position] path to location in + modulestore. The courseware insists that the first level in the course is + chapter, but any kind of module can be a "section". + + location: something that can be passed to Location + course_name: [optional]. If not None, restrict search to paths + in that course. + + raise ItemNotFoundError if the location doesn't exist. + + raise NoPathToItem if the location exists, but isn't accessible via + a chapter/section path in the course(s) being searched. + + Return a tuple (course_id, chapter, section, position) suitable for the + courseware index view. + + A location may be accessible via many paths. This method may + return any valid path. + + If the section is a sequence, position will be the position + of this location in that sequence. Otherwise, position will + be None. TODO (vshnayder): Not true yet. + ''' + + def flatten(xs): + '''Convert lisp-style (a, (b, (c, ()))) list into a python list. + Not a general flatten function. ''' + p = [] + while xs != (): + p.append(xs[0]) + xs = xs[1] + return p + + def find_path_to_course(location, course_name=None): + '''Find a path up the location graph to a node with the + specified category. + + If no path exists, return None. + + If a path exists, return it as a list with target location first, and + the starting location last. + ''' + # Standard DFS + + # To keep track of where we came from, the work queue has + # tuples (location, path-so-far). To avoid lots of + # copying, the path-so-far is stored as a lisp-style + # list--nested hd::tl tuples, and flattened at the end. + queue = [(location, ())] + while len(queue) > 0: + (loc, path) = queue.pop() # Takes from the end + loc = Location(loc) + + # get_parent_locations should raise ItemNotFoundError if location + # isn't found so we don't have to do it explicitly. Call this + # first to make sure the location is there (even if it's a course, and + # we would otherwise immediately exit). + parents = modulestore.get_parent_locations(loc) + + # print 'Processing loc={0}, path={1}'.format(loc, path) + if loc.category == "course": + if course_name is None or course_name == loc.name: + # Found it! + path = (loc, path) + return flatten(path) + + # otherwise, add parent locations at the end + newpath = (loc, path) + queue.extend(zip(parents, repeat(newpath))) + + # If we're here, there is no path + return None + + path = find_path_to_course(location, course_name) + if path is None: + raise(NoPathToItem(location)) + + n = len(path) + course_id = CourseDescriptor.location_to_id(path[0]) + chapter = path[1].name if n > 1 else None + section = path[2].name if n > 2 else None + + # TODO (vshnayder): not handling position at all yet... + position = None + + return (course_id, chapter, section, position) diff --git a/common/lib/xmodule/xmodule/modulestore/tests/test_mongo.py b/common/lib/xmodule/xmodule/modulestore/tests/test_mongo.py index cb2bc6e20c..24f0441ee0 100644 --- a/common/lib/xmodule/xmodule/modulestore/tests/test_mongo.py +++ b/common/lib/xmodule/xmodule/modulestore/tests/test_mongo.py @@ -8,6 +8,7 @@ from xmodule.modulestore import Location from xmodule.modulestore.exceptions import InvalidLocationError, ItemNotFoundError, NoPathToItem from xmodule.modulestore.mongo import MongoModuleStore from xmodule.modulestore.xml_importer import import_from_xml +from xmodule.modulestore.search import path_to_location # from ~/mitx_all/mitx/common/lib/xmodule/xmodule/modulestore/tests/ # to ~/mitx_all/mitx/common/test @@ -28,7 +29,7 @@ DEFAULT_CLASS = 'xmodule.raw_module.RawDescriptor' class TestMongoModuleStore(object): - + '''Tests!''' @classmethod def setupClass(cls): cls.connection = pymongo.connection.Connection(HOST, PORT) @@ -67,7 +68,7 @@ class TestMongoModuleStore(object): def test_init(self): '''Make sure the db loads, and print all the locations in the db. - Call this directly from failing tests to see what's loaded''' + Call this directly from failing tests to see what is loaded''' ids = list(self.connection[DB][COLLECTION].find({}, {'_id': True})) pprint([Location(i['_id']).url() for i in ids]) @@ -93,8 +94,6 @@ class TestMongoModuleStore(object): self.store.get_item("i4x://edX/toy/video/Welcome"), None) - - def test_find_one(self): assert_not_equals( self.store._find_one(Location("i4x://edX/toy/course/2012_Fall")), @@ -117,13 +116,13 @@ class TestMongoModuleStore(object): ("edX/toy/2012_Fall", "Overview", "Toy_Videos", None)), ) for location, expected in should_work: - assert_equals(self.store.path_to_location(location), expected) + assert_equals(path_to_location(self.store, location), expected) not_found = ( - "i4x://edX/toy/video/WelcomeX", + "i4x://edX/toy/video/WelcomeX", "i4x://edX/toy/course/NotHome" ) for location in not_found: - assert_raises(ItemNotFoundError, self.store.path_to_location, location) + assert_raises(ItemNotFoundError, path_to_location, self.store, location) # Since our test files are valid, there shouldn't be any # elements with no path to them. But we can look for them in @@ -132,5 +131,5 @@ class TestMongoModuleStore(object): "i4x://edX/simple/video/Lost_Video", ) for location in no_path: - assert_raises(NoPathToItem, self.store.path_to_location, location, "toy") + assert_raises(NoPathToItem, path_to_location, self.store, location, "toy") diff --git a/common/lib/xmodule/xmodule/modulestore/xml.py b/common/lib/xmodule/xmodule/modulestore/xml.py index 7dd6868f78..e7f9c9ce0a 100644 --- a/common/lib/xmodule/xmodule/modulestore/xml.py +++ b/common/lib/xmodule/xmodule/modulestore/xml.py @@ -1,14 +1,16 @@ import logging +import os +import re + from fs.osfs import OSFS from importlib import import_module from lxml import etree from path import path -from xmodule.errorhandlers import logging_error_handler +from xmodule.errortracker import ErrorLog, make_error_tracker from xmodule.x_module import XModuleDescriptor, XMLParsingSystem +from xmodule.course_module import CourseDescriptor from xmodule.mako_module import MakoDescriptorSystem from cStringIO import StringIO -import os -import re from . import ModuleStore, Location from .exceptions import ItemNotFoundError @@ -19,7 +21,6 @@ etree.set_default_parser( log = logging.getLogger('mitx.' + __name__) - # VS[compat] # TODO (cpennington): Remove this once all fall 2012 courses have been imported # into the cms from xml @@ -29,7 +30,7 @@ def clean_out_mako_templating(xml_string): return xml_string class ImportSystem(XMLParsingSystem, MakoDescriptorSystem): - def __init__(self, xmlstore, org, course, course_dir, error_handler): + def __init__(self, xmlstore, org, course, course_dir, error_tracker, **kwargs): """ A class that handles loading from xml. Does some munging to ensure that all elements have unique slugs. @@ -70,28 +71,28 @@ class ImportSystem(XMLParsingSystem, MakoDescriptorSystem): # log.debug('-> slug=%s' % slug) xml_data.set('url_name', slug) - module = XModuleDescriptor.load_from_xml( + descriptor = XModuleDescriptor.load_from_xml( etree.tostring(xml_data), self, org, course, xmlstore.default_class) - #log.debug('==> importing module location %s' % repr(module.location)) - module.metadata['data_dir'] = course_dir + #log.debug('==> importing descriptor location %s' % + # repr(descriptor.location)) + descriptor.metadata['data_dir'] = course_dir - xmlstore.modules[module.location] = module + xmlstore.modules[descriptor.location] = descriptor if xmlstore.eager: - module.get_children() - return module + descriptor.get_children() + return descriptor render_template = lambda: '' load_item = xmlstore.get_item resources_fs = OSFS(xmlstore.data_dir / course_dir) MakoDescriptorSystem.__init__(self, load_item, resources_fs, - error_handler, render_template) + error_tracker, render_template, **kwargs) XMLParsingSystem.__init__(self, load_item, resources_fs, - error_handler, process_xml) - + error_tracker, process_xml, **kwargs) class XMLModuleStore(ModuleStore): @@ -99,8 +100,7 @@ class XMLModuleStore(ModuleStore): An XML backed ModuleStore """ def __init__(self, data_dir, default_class=None, eager=False, - course_dirs=None, - error_handler=logging_error_handler): + course_dirs=None): """ Initialize an XMLModuleStore from data_dir @@ -114,17 +114,14 @@ class XMLModuleStore(ModuleStore): course_dirs: If specified, the list of course_dirs to load. Otherwise, load all course dirs - - error_handler: The error handler used here and in the underlying - DescriptorSystem. By default, raise exceptions for all errors. - See the comments in x_module.py:DescriptorSystem """ self.eager = eager self.data_dir = path(data_dir) self.modules = {} # location -> XModuleDescriptor self.courses = {} # course_dir -> XModuleDescriptor for the course - self.error_handler = error_handler + self.location_errors = {} # location -> ErrorLog + if default_class is None: self.default_class = None @@ -148,15 +145,18 @@ class XMLModuleStore(ModuleStore): for course_dir in course_dirs: try: - course_descriptor = self.load_course(course_dir) + # make a tracker, then stick in the right place once the course loads + # and we know its location + errorlog = make_error_tracker() + course_descriptor = self.load_course(course_dir, errorlog.tracker) self.courses[course_dir] = course_descriptor + self.location_errors[course_descriptor.location] = errorlog except: msg = "Failed to load course '%s'" % course_dir log.exception(msg) - error_handler(msg) - def load_course(self, course_dir): + def load_course(self, course_dir, tracker): """ Load a course into this module store course_path: Course directory name @@ -190,13 +190,13 @@ class XMLModuleStore(ModuleStore): )) course = course_dir - system = ImportSystem(self, org, course, course_dir, - self.error_handler) + system = ImportSystem(self, org, course, course_dir, tracker) course_descriptor = system.process_xml(etree.tostring(course_data)) log.debug('========> Done with course import from {0}'.format(course_dir)) return course_descriptor + def get_item(self, location, depth=0): """ Returns an XModuleDescriptor instance for the item at location. @@ -217,9 +217,28 @@ class XMLModuleStore(ModuleStore): except KeyError: raise ItemNotFoundError(location) + + def get_item_errors(self, location): + """ + Return list of errors for this location, if any. Raise the same + errors as get_item if location isn't present. + + NOTE: This only actually works for courses in the xml datastore-- + will return an empty list for all other modules. + """ + location = Location(location) + # check that item is present + self.get_item(location) + + # now look up errors + if location in self.location_errors: + return self.location_errors[location].errors + return [] + def get_courses(self, depth=0): """ - Returns a list of course descriptors + Returns a list of course descriptors. If there were errors on loading, + some of these may be ErrorDescriptors instead. """ return self.courses.values() diff --git a/common/lib/xmodule/xmodule/raw_module.py b/common/lib/xmodule/xmodule/raw_module.py index 90f4139bd5..9fdb5d0b38 100644 --- a/common/lib/xmodule/xmodule/raw_module.py +++ b/common/lib/xmodule/xmodule/raw_module.py @@ -1,27 +1,16 @@ -from pkg_resources import resource_string from lxml import etree -from xmodule.mako_module import MakoModuleDescriptor +from xmodule.editing_module import EditingDescriptor from xmodule.xml_module import XmlDescriptor import logging +import sys log = logging.getLogger(__name__) - -class RawDescriptor(MakoModuleDescriptor, XmlDescriptor): +class RawDescriptor(XmlDescriptor, EditingDescriptor): """ - Module that provides a raw editing view of its data and children + Module that provides a raw editing view of its data and children. It + requires that the definition xml is valid. """ - mako_template = "widgets/raw-edit.html" - - js = {'coffee': [resource_string(__name__, 'js/src/raw/edit.coffee')]} - js_module_name = "RawDescriptor" - - def get_context(self): - return { - 'module': self, - 'data': self.definition['data'], - } - @classmethod def definition_from_xml(cls, xml_object, system): return {'data': etree.tostring(xml_object)} @@ -30,13 +19,12 @@ class RawDescriptor(MakoModuleDescriptor, XmlDescriptor): try: return etree.fromstring(self.definition['data']) except etree.XMLSyntaxError as err: + # Can't recover here, so just add some info and + # re-raise lines = self.definition['data'].split('\n') line, offset = err.position msg = ("Unable to create xml for problem {loc}. " "Context: '{context}'".format( context=lines[line - 1][offset - 40:offset + 40], loc=self.location)) - log.exception(msg) - self.system.error_handler(msg) - # no workaround possible, so just re-raise - raise + raise Exception, msg, sys.exc_info()[2] diff --git a/common/lib/xmodule/xmodule/template_module.py b/common/lib/xmodule/xmodule/template_module.py index 3f926555f4..260ad009f9 100644 --- a/common/lib/xmodule/xmodule/template_module.py +++ b/common/lib/xmodule/xmodule/template_module.py @@ -7,16 +7,14 @@ from mako.template import Template class CustomTagModule(XModule): """ This module supports tags of the form -
${err}We're sorry, this module is temporarily unavailable. Our staff is working to fix it as soon as possible. Please email us at technical@mitx.mit.edu to report any problems or downtime.
+ +Error: ${error}
+ +Raw data: ${data}
+ +