})
+ })
+ })
+ })
+
+
+
+
+ %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/css/sequence/display.scss b/common/lib/xmodule/xmodule/css/sequence/display.scss index 7658797725..6e7c5d24a7 100644 --- a/common/lib/xmodule/xmodule/css/sequence/display.scss +++ b/common/lib/xmodule/xmodule/css/sequence/display.scss @@ -2,10 +2,8 @@ nav.sequence-nav { // TODO (cpennington): This doesn't work anymore. XModules aren't able to // import from external sources. @extend .topbar; - - border-bottom: 1px solid #ddd; + border-bottom: 1px solid $border-color; margin: (-(lh())) (-(lh())) lh() (-(lh())); - background: #eee; position: relative; @include border-top-right-radius(4px); @@ -14,7 +12,7 @@ nav.sequence-nav { display: table; height: 100%; margin: 0; - padding-left: 0; + padding-left: 3px; padding-right: flex-grid(1, 9); width: 100%; @@ -23,133 +21,104 @@ nav.sequence-nav { } li { - border-left: 1px solid #eee; display: table-cell; min-width: 20px; - &:first-child { - border-left: none; - } - - .inactive { - background-repeat: no-repeat; - - &:hover { - background-color: #eee; - } - } - - .visited { - background-color: #ddd; - background-repeat: no-repeat; - - &:hover { - background-position: center center; - } - } - - .active { - background-color: #fff; - background-repeat: no-repeat; - @include box-shadow(0 1px 0 #fff); - - &:hover { - background-color: #fff; - background-position: center; - } - } - a { - background-position: center center; - border: none; + background-position: center; + background-repeat: no-repeat; + border: 1px solid transparent; + border-bottom: none; + @include border-radius(3px 3px 0 0); cursor: pointer; display: block; - height: 17px; + height: 10px; padding: 15px 0 14px; position: relative; - @include transition(all, .4s, $ease-in-out-quad); + @include transition(); width: 100%; - &.progress { - border-bottom-style: solid; - border-bottom-width: 4px; + &:hover { + background-repeat: no-repeat; + background-position: center; + background-color: #F6F6F6; + } + + &.visited { + background-color: #F6F6F6; + + &:hover { + background-position: center center; + } + } + + &.active { + border-color: $border-color; + @include box-shadow(0 2px 0 #fff); + background-color: #fff; + z-index: 9; + + &:hover { + background-position: center; + background-color: #fff; + } } &.progress-none { - @extend .progress; - border-bottom-color: red; + background-color: lighten(red, 50%); } &.progress-some { - @extend .progress; - border-bottom-color: yellow; + background-color: yellow; } &.progress-done { - @extend .progress; - border-bottom-color: green; + background-color: green; } //video &.seq_video { &.inactive { - @extend .inactive; background-image: url('../images/sequence-nav/video-icon-normal.png'); - background-position: center; } &.visited { - @extend .visited; background-image: url('../images/sequence-nav/video-icon-visited.png'); - background-position: center; } &.active { @extend .active; background-image: url('../images/sequence-nav/video-icon-current.png'); - background-position: center; } } //other &.seq_other { &.inactive { - @extend .inactive; background-image: url('../images/sequence-nav/document-icon-normal.png'); - background-position: center; } &.visited { - @extend .visited; background-image: url('../images/sequence-nav/document-icon-visited.png'); - background-position: center; } &.active { - @extend .active; background-image: url('../images/sequence-nav/document-icon-current.png'); - background-position: center; } } //vertical & problems &.seq_vertical, &.seq_problem { &.inactive { - @extend .inactive; background-image: url('../images/sequence-nav/list-icon-normal.png'); - background-position: center; } &.visited { - @extend .visited; background-image: url('../images/sequence-nav/list-icon-visited.png'); - background-position: center; } &.active { - @extend .active; background-image: url('../images/sequence-nav/list-icon-current.png'); - background-position: center; } } @@ -157,6 +126,7 @@ nav.sequence-nav { background: #333; color: #fff; display: none; + font-family: $sans-serif; line-height: lh(); left: 0px; opacity: 0; @@ -207,27 +177,29 @@ nav.sequence-nav { right: 0; top: 0; width: flex-grid(1, 9); + border: 1px solid $border-color; + border-bottom: 0; + @include border-radius(3px 3px 0 0); li { float: left; + margin-bottom: 0; width: 50%; &.prev, &.next { a { - // background-color: darken($cream, 5%); - background-position: center center; + background-position: center; background-repeat: no-repeat; - border-left: 1px solid darken(#f6efd4, 20%); - @include box-shadow(inset 1px 0 0 lighten(#f6efd4, 5%)); - @include box-sizing(border-box); - cursor: pointer; display: block; + height: 10px; + padding: 15px 0 14px; text-indent: -9999px; @include transition(all, .2s, $ease-in-out-quad); &:hover { opacity: .5; + background-color: #f4f4f4; } &.disabled { @@ -240,20 +212,13 @@ nav.sequence-nav { &.prev { a { background-image: url('../images/sequence-nav/previous-icon.png'); - - &:hover { - // background-color: $cream; - } } } &.next { a { + border-left: 1px solid lighten($border-color, 10%); background-image: url('../images/sequence-nav/next-icon.png'); - - &:hover { - // background-color: $cream; - } } } } @@ -274,10 +239,8 @@ nav.sequence-bottom { ul { @extend .clearfix; - background-color: #eee; - border: 1px solid #ddd; + border: 1px solid $border-color; @include border-radius(3px); - @include box-shadow(inset 0 0 0 1px lighten(#f6efd4, 5%)); @include inline-block(); li { @@ -312,7 +275,7 @@ nav.sequence-bottom { &.prev { a { background-image: url('../images/sequence-nav/previous-icon.png'); - border-right: 1px solid darken(#f6efd4, 20%); + border-right: 1px solid lighten($border-color, 10%); &:hover { background-color: none; diff --git a/common/lib/xmodule/xmodule/editing_module.py b/common/lib/xmodule/xmodule/editing_module.py new file mode 100644 index 0000000000..67a4d66dad --- /dev/null +++ b/common/lib/xmodule/xmodule/editing_module.py @@ -0,0 +1,29 @@ +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.get('data', ''), + # TODO (vshnayder): allow children and metadata to be edited. + #'children' : self.definition.get('children, ''), + + # TODO: show both own metadata and inherited? + #'metadata' : self.own_metadata, + } diff --git a/common/lib/xmodule/xmodule/error_module.py b/common/lib/xmodule/xmodule/error_module.py new file mode 100644 index 0000000000..ecc90873b9 --- /dev/null +++ b/common/lib/xmodule/xmodule/error_module.py @@ -0,0 +1,89 @@ +import sys +import logging + +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 +from xmodule.errortracker import exc_info_to_str + + +log = logging.getLogger(__name__) + +class ErrorModule(XModule): + def get_html(self): + '''Show an error. + TODO (vshnayder): proper style, divs, etc. + ''' + # staff get to see all the details + return self.system.render_template('module-error.html', { + 'data' : self.definition['data']['contents'], + 'error' : self.definition['data']['error_msg'], + 'is_staff' : self.system.is_staff, + }) + +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, + error_msg='Error not available'): + '''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. (should be a string, or convert usefully into one). + ''' + # Use a nested inner dictionary because 'data' is hardcoded + inner = {} + definition = {'data': inner} + inner['error_msg'] = str(error_msg) + + 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 + error_node = xml_obj.find('error_msg') + if error_node is not None: + inner['error_msg'] = error_node.text + else: + inner['error_msg'] = 'Error not available' + + except etree.XMLSyntaxError: + # Save the error to display later--overrides other problems + inner['error_msg'] = exc_info_to_str(sys.exc_info()) + + inner['contents'] = 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']['contents']) + return etree.tostring(xml) + except etree.XMLSyntaxError: + # still not valid. + root = etree.Element('error') + root.text = self.definition['data']['contents'] + err_node = etree.SubElement(root, 'error_msg') + err_node.text = self.definition['data']['error_msg'] + 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..8ac2903149 --- /dev/null +++ b/common/lib/xmodule/xmodule/errortracker.py @@ -0,0 +1,44 @@ +import logging +import sys +import traceback + +from collections import namedtuple + +log = logging.getLogger(__name__) + +ErrorLog = namedtuple('ErrorLog', 'tracker errors') + +def exc_info_to_str(exc_info): + """Given some exception info, convert it into a string using + the traceback.format_exception() function. + """ + return ''.join(traceback.format_exception(*exc_info)) + +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 = exc_info_to_str(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_checker.py b/common/lib/xmodule/xmodule/html_checker.py new file mode 100644 index 0000000000..5e6b417d28 --- /dev/null +++ b/common/lib/xmodule/xmodule/html_checker.py @@ -0,0 +1,14 @@ +from lxml import etree + +def check_html(html): + ''' + Check whether the passed in html string can be parsed by lxml. + Return bool success. + ''' + parser = etree.HTMLParser() + try: + etree.fromstring(html, parser) + return True + except Exception as err: + pass + return False diff --git a/common/lib/xmodule/xmodule/html_module.py b/common/lib/xmodule/xmodule/html_module.py index b9bc34aed6..7a09004e33 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 .x_module import XModule +from .xml_module import XmlDescriptor +from .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.url_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.url_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..e59e4bd68e 100644 --- a/common/lib/xmodule/xmodule/modulestore/__init__.py +++ b/common/lib/xmodule/xmodule/modulestore/__init__.py @@ -3,10 +3,13 @@ This module provides an abstraction for working with XModuleDescriptors that are stored in a database an accessible using their Location as an identifier """ -import re -from collections import namedtuple -from .exceptions import InvalidLocationError import logging +import re + +from collections import namedtuple + +from .exceptions import InvalidLocationError, InsufficientSpecificationError +from xmodule.errortracker import ErrorLog, make_error_tracker log = logging.getLogger('mitx.' + 'modulestore') @@ -38,15 +41,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 +59,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 +216,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 +284,47 @@ 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 + +class ModuleStoreBase(ModuleStore): + ''' + Implement interface functionality that can be shared. + ''' + def __init__(self): + ''' + Set up the error-tracking logic. + ''' + self._location_errors = {} # location -> ErrorLog + + def _get_errorlog(self, location): + """ + If we already have an errorlog for this location, return it. Otherwise, + create one. + """ + location = Location(location) + if location not in self._location_errors: + self._location_errors[location] = make_error_tracker() + return self._location_errors[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: For now, the only items that track errors are CourseDescriptors in + the xml datastore. This will return an empty list for all other items + and datastores. + """ + # check that item is present and raise the promised exceptions if needed + self.get_item(location) + + errorlog = self._get_errorlog(location) + return errorlog.errors diff --git a/common/lib/xmodule/xmodule/modulestore/mongo.py b/common/lib/xmodule/xmodule/modulestore/mongo.py index df4e20f3a7..1cec6c7f87 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 . import ModuleStoreBase, Location +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: a function that logs errors for later display to users 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 @@ -74,13 +73,17 @@ def location_to_query(location): return query -class MongoModuleStore(ModuleStore): +class MongoModuleStore(ModuleStoreBase): """ A Mongodb backed 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): + + ModuleStoreBase.__init__(self) + self.collection = pymongo.connection.Connection( host=host, port=port @@ -91,13 +94,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 +156,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 +179,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 +209,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 +288,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..baf3d46b57 --- /dev/null +++ b/common/lib/xmodule/xmodule/modulestore/search.py @@ -0,0 +1,97 @@ +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]) + # pull out the location names + 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..0567e4e7a7 100644 --- a/common/lib/xmodule/xmodule/modulestore/xml.py +++ b/common/lib/xmodule/xmodule/modulestore/xml.py @@ -1,16 +1,18 @@ 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 . import ModuleStoreBase, Location from .exceptions import ItemNotFoundError etree.set_default_parser( @@ -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. @@ -40,6 +41,9 @@ class ImportSystem(XMLParsingSystem, MakoDescriptorSystem): self.used_slugs = set() def process_xml(xml): + """Takes an xml string, and returns a XModuleDescriptor created from + that xml. + """ try: # VS[compat] # TODO (cpennington): Remove this once all fall 2012 courses @@ -70,37 +74,36 @@ 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): +class XMLModuleStore(ModuleStoreBase): """ 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 +117,13 @@ 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 """ + ModuleStoreBase.__init__(self) 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 if default_class is None: self.default_class = None @@ -148,15 +147,20 @@ class XMLModuleStore(ModuleStore): for course_dir in course_dirs: try: - course_descriptor = self.load_course(course_dir) + # Special-case code here, since we don't have a location for the + # course before it loads. + # So, make a tracker to track load-time errors, then put in the right + # place after the course loads and we have 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 +194,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,15 +221,19 @@ class XMLModuleStore(ModuleStore): except KeyError: raise ItemNotFoundError(location) + 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() + def create_item(self, location): raise NotImplementedError("XMLModuleStores are read-only") + def update_item(self, location, data): """ Set the data in the item specified by the location to @@ -236,6 +244,7 @@ class XMLModuleStore(ModuleStore): """ raise NotImplementedError("XMLModuleStores are read-only") + def update_children(self, location, children): """ Set the children for the item specified by the location to @@ -246,6 +255,7 @@ class XMLModuleStore(ModuleStore): """ raise NotImplementedError("XMLModuleStores are read-only") + def update_metadata(self, location, metadata): """ Set the metadata for the item specified by the location to diff --git a/common/lib/xmodule/xmodule/modulestore/xml_importer.py b/common/lib/xmodule/xmodule/modulestore/xml_importer.py index 578ade95fe..891db7e994 100644 --- a/common/lib/xmodule/xmodule/modulestore/xml_importer.py +++ b/common/lib/xmodule/xmodule/modulestore/xml_importer.py @@ -35,6 +35,8 @@ def import_from_xml(store, data_dir, course_dirs=None, eager=True, store.update_item(module.location, module.definition['data']) if 'children' in module.definition: store.update_children(module.location, module.definition['children']) - store.update_metadata(module.location, dict(module.metadata)) + # NOTE: It's important to use own_metadata here to avoid writing + # inherited metadata everywhere. + store.update_metadata(module.location, dict(module.own_metadata)) return module_store 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/stringify.py b/common/lib/xmodule/xmodule/stringify.py new file mode 100644 index 0000000000..dad964140f --- /dev/null +++ b/common/lib/xmodule/xmodule/stringify.py @@ -0,0 +1,20 @@ +from itertools import chain +from lxml import etree + +def stringify_children(node): + ''' + Return all contents of an xml tree, without the outside tags. + e.g. if node is parse of + "Hi
there Bruce!
"
+ should return
+ "Hi there Bruce!
"
+
+ fixed from
+ http://stackoverflow.com/questions/4624062/get-all-text-inside-a-tag-in-lxml
+ '''
+ parts = ([node.text] +
+ list(chain(*([etree.tostring(c), c.tail]
+ for c in node.getchildren())
+ )))
+ # filter removes possible Nones in texts and tails
+ return ''.join(filter(None, parts))
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
- there Bruce!
'
+ html = '''{0}'''.format(text)
+ xml = etree.fromstring(html)
+ out = stringify_children(xml)
+ assert_equals(out, text)
diff --git a/common/lib/xmodule/xmodule/video_module.py b/common/lib/xmodule/xmodule/video_module.py
index da10e4bc91..fb68ba982b 100644
--- a/common/lib/xmodule/xmodule/video_module.py
+++ b/common/lib/xmodule/xmodule/video_module.py
@@ -23,11 +23,12 @@ class VideoModule(XModule):
css = {'scss': [resource_string(__name__, 'css/video/display.scss')]}
js_module_name = "Video"
- def __init__(self, system, location, definition, instance_state=None, shared_state=None, **kwargs):
- XModule.__init__(self, system, location, definition, instance_state, shared_state, **kwargs)
+ def __init__(self, system, location, definition,
+ instance_state=None, shared_state=None, **kwargs):
+ XModule.__init__(self, system, location, definition,
+ instance_state, shared_state, **kwargs)
xmltree = etree.fromstring(self.definition['data'])
self.youtube = xmltree.get('youtube')
- self.name = xmltree.get('name')
self.position = 0
if instance_state is not None:
@@ -71,7 +72,7 @@ class VideoModule(XModule):
'streams': self.video_list(),
'id': self.location.html_id(),
'position': self.position,
- 'name': self.name,
+ 'display_name': self.display_name,
# TODO (cpennington): This won't work when we move to data that isn't on the filesystem
'data_dir': self.metadata['data_dir'],
})
diff --git a/common/lib/xmodule/xmodule/x_module.py b/common/lib/xmodule/xmodule/x_module.py
index 30522b871c..e5508d0e3b 100644
--- a/common/lib/xmodule/xmodule/x_module.py
+++ b/common/lib/xmodule/xmodule/x_module.py
@@ -1,10 +1,14 @@
-from lxml import etree
-import pkg_resources
import logging
+import pkg_resources
+import sys
+
+from fs.errors import ResourceNotFoundError
+from functools import partial
+from lxml import etree
+from lxml.etree import XMLSyntaxError
from xmodule.modulestore import Location
-
-from functools import partial
+from xmodule.errortracker import exc_info_to_str
log = logging.getLogger('mitx.' + __name__)
@@ -187,13 +191,19 @@ class XModule(HTMLSnippet):
self.instance_state = instance_state
self.shared_state = shared_state
self.id = self.location.url()
- self.name = self.location.name
+ self.url_name = self.location.name
self.category = self.location.category
self.metadata = kwargs.get('metadata', {})
self._loaded_children = None
- def get_name(self):
- return self.name
+ @property
+ def display_name(self):
+ '''
+ Return a display name for the module: use display_name if defined in
+ metadata, otherwise convert the url name.
+ '''
+ return self.metadata.get('display_name',
+ self.url_name.replace('_', ' '))
def get_children(self):
'''
@@ -338,6 +348,8 @@ class XModuleDescriptor(Plugin, HTMLSnippet):
module
display_name: The name to use for displaying this module to the
user
+ url_name: The name to use for this module in urls and other places
+ where a unique name is needed.
format: The format of this module ('Homework', 'Lab', etc)
graded (bool): Whether this module is should be graded or not
start (string): The date for which this module will be available
@@ -352,13 +364,30 @@ class XModuleDescriptor(Plugin, HTMLSnippet):
self.metadata = kwargs.get('metadata', {})
self.definition = definition if definition is not None else {}
self.location = Location(kwargs.get('location'))
- self.name = self.location.name
+ self.url_name = self.location.name
self.category = self.location.category
self.shared_state_key = kwargs.get('shared_state_key')
self._child_instances = None
self._inherited_metadata = set()
+ @property
+ def display_name(self):
+ '''
+ Return a display name for the module: use display_name if defined in
+ metadata, otherwise convert the url name.
+ '''
+ return self.metadata.get('display_name',
+ self.url_name.replace('_', ' '))
+
+ @property
+ def own_metadata(self):
+ """
+ Return the metadata that is not inherited, but was defined on this module.
+ """
+ return dict((k,v) for k,v in self.metadata.items()
+ if k not in self._inherited_metadata)
+
def inherit_metadata(self, metadata):
"""
Updates this module with metadata inherited from a containing module.
@@ -443,16 +472,32 @@ class XModuleDescriptor(Plugin, HTMLSnippet):
system is an XMLParsingSystem
org and course are optional strings that will be used in the generated
- modules url identifiers
+ module's url identifiers
"""
- class_ = XModuleDescriptor.load_class(
- etree.fromstring(xml_data).tag,
- default_class
- )
- # leave next line, commented out - useful for low-level debugging
- # log.debug('[XModuleDescriptor.load_from_xml] tag=%s, class_=%s' % (
- # etree.fromstring(xml_data).tag,class_))
- return class_.from_xml(xml_data, system, org, course)
+ try:
+ class_ = XModuleDescriptor.load_class(
+ etree.fromstring(xml_data).tag,
+ default_class
+ )
+ # leave next line, commented out - useful for low-level debugging
+ # log.debug('[XModuleDescriptor.load_from_xml] tag=%s, class_=%s' % (
+ # etree.fromstring(xml_data).tag,class_))
+
+ descriptor = class_.from_xml(xml_data, system, org, course)
+ except Exception as err:
+ # Didn't load properly. Fall back on loading as an error
+ # descriptor. This should never error due to formatting.
+
+ # Put import here to avoid circular import errors
+ from xmodule.error_module import ErrorDescriptor
+ msg = "Error loading from xml."
+ log.exception(msg)
+ system.error_tracker(msg)
+ err_msg = msg + "\n" + exc_info_to_str(sys.exc_info())
+ descriptor = ErrorDescriptor.from_xml(xml_data, system, org, course,
+ err_msg)
+
+ return descriptor
@classmethod
def from_xml(cls, xml_data, system, org=None, course=None):
@@ -521,16 +566,19 @@ class XModuleDescriptor(Plugin, HTMLSnippet):
class DescriptorSystem(object):
- def __init__(self, load_item, resources_fs, error_handler):
+ def __init__(self, load_item, resources_fs, error_tracker, **kwargs):
"""
load_item: Takes a Location and returns an XModuleDescriptor
resources_fs: A Filesystem object that contains all of the
resources needed for the course
- error_handler: A hook for handling errors in loading the descriptor.
- Must be a function of (error_msg, exc_info=None).
- See errorhandlers.py for some simple ones.
+ error_tracker: A hook for tracking errors in loading the descriptor.
+ Used for example to get a list of all non-fatal problems on course
+ load, and display them to the user.
+
+ A function of (error_msg). errortracker.py provides a
+ handy make_error_tracker() function.
Patterns for using the error handler:
try:
@@ -539,10 +587,8 @@ class DescriptorSystem(object):
except SomeProblem:
msg = 'Grommet {0} is broken'.format(x)
log.exception(msg) # don't rely on handler to log
- self.system.error_handler(msg)
- # if we get here, work around if possible
- raise # if no way to work around
- OR
+ self.system.error_tracker(msg)
+ # work around
return 'Oops, couldn't load grommet'
OR, if not in an exception context:
@@ -550,25 +596,27 @@ class DescriptorSystem(object):
if not check_something(thingy):
msg = "thingy {0} is broken".format(thingy)
log.critical(msg)
- error_handler(msg)
- # if we get here, work around
- pass # e.g. if no workaround needed
+ self.system.error_tracker(msg)
+
+ NOTE: To avoid duplication, do not call the tracker on errors
+ that you're about to re-raise---let the caller track them.
"""
self.load_item = load_item
self.resources_fs = resources_fs
- self.error_handler = error_handler
+ self.error_tracker = error_tracker
class XMLParsingSystem(DescriptorSystem):
- def __init__(self, load_item, resources_fs, error_handler, process_xml):
+ def __init__(self, load_item, resources_fs, error_tracker, process_xml, **kwargs):
"""
- load_item, resources_fs, error_handler: see DescriptorSystem
+ load_item, resources_fs, error_tracker: see DescriptorSystem
process_xml: Takes an xml string, and returns a XModuleDescriptor
created from that xml
"""
- DescriptorSystem.__init__(self, load_item, resources_fs, error_handler)
+ DescriptorSystem.__init__(self, load_item, resources_fs, error_tracker,
+ **kwargs)
self.process_xml = process_xml
@@ -584,10 +632,18 @@ class ModuleSystem(object):
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):
+ def __init__(self,
+ ajax_url,
+ track_function,
+ get_module,
+ render_template,
+ replace_urls,
+ user=None,
+ filestore=None,
+ debug=False,
+ xqueue_callback_url=None,
+ xqueue_default_queuename="null",
+ is_staff=False):
'''
Create a closure around the system environment.
@@ -613,9 +669,13 @@ class ModuleSystem(object):
replace_urls - TEMPORARY - A function like static_replace.replace_urls
that capa_module can use to fix up the static urls in
ajax results.
+
+ is_staff - Is the user making the request a staff user?
+ TODO (vshnayder): this will need to change once we have real user roles.
'''
self.ajax_url = ajax_url
self.xqueue_callback_url = xqueue_callback_url
+ self.xqueue_default_queuename = xqueue_default_queuename
self.track_function = track_function
self.filestore = filestore
self.get_module = get_module
@@ -623,6 +683,7 @@ class ModuleSystem(object):
self.DEBUG = self.debug = debug
self.seed = user.id if user is not None else 0
self.replace_urls = replace_urls
+ self.is_staff = is_staff
def get(self, attr):
''' provide uniform access to attributes (like etree).'''
diff --git a/common/lib/xmodule/xmodule/xml_module.py b/common/lib/xmodule/xmodule/xml_module.py
index 6750906eb4..b0a289d149 100644
--- a/common/lib/xmodule/xmodule/xml_module.py
+++ b/common/lib/xmodule/xmodule/xml_module.py
@@ -1,4 +1,3 @@
-from collections import MutableMapping
from xmodule.x_module import XModuleDescriptor
from xmodule.modulestore import Location
from lxml import etree
@@ -8,74 +7,12 @@ import traceback
from collections import namedtuple
from fs.errors import ResourceNotFoundError
import os
+import sys
log = logging.getLogger(__name__)
-# TODO (cpennington): This was implemented in an attempt to improve performance,
-# but the actual improvement wasn't measured (and it was implemented late at night).
-# We should check if it hurts, and whether there's a better way of doing lazy loading
-
-
-class LazyLoadingDict(MutableMapping):
- """
- A dictionary object that lazily loads its contents from a provided
- function on reads (of members that haven't already been set).
- """
-
- def __init__(self, loader):
- '''
- On the first read from this dictionary, it will call loader() to
- populate its contents. loader() must return something dict-like. Any
- elements set before the first read will be preserved.
- '''
- self._contents = {}
- self._loaded = False
- self._loader = loader
- self._deleted = set()
-
- def __getitem__(self, name):
- if not (self._loaded or name in self._contents or name in self._deleted):
- self.load()
-
- return self._contents[name]
-
- def __setitem__(self, name, value):
- self._contents[name] = value
- self._deleted.discard(name)
-
- def __delitem__(self, name):
- del self._contents[name]
- self._deleted.add(name)
-
- def __contains__(self, name):
- self.load()
- return name in self._contents
-
- def __len__(self):
- self.load()
- return len(self._contents)
-
- def __iter__(self):
- self.load()
- return iter(self._contents)
-
- def __repr__(self):
- self.load()
- return repr(self._contents)
-
- def load(self):
- if self._loaded:
- return
-
- loaded_contents = self._loader()
- loaded_contents.update(self._contents)
- self._contents = loaded_contents
- self._loaded = True
-
-
_AttrMapBase = namedtuple('_AttrMap', 'metadata_key to_metadata from_metadata')
-
class AttrMap(_AttrMapBase):
"""
A class that specifies a metadata_key, and two functions:
@@ -163,6 +100,45 @@ class XmlDescriptor(XModuleDescriptor):
"""
return etree.parse(file_object).getroot()
+ @classmethod
+ def load_definition(cls, xml_object, system, location):
+ '''Load a descriptor definition from the specified xml_object.
+ Subclasses should not need to override this except in special
+ cases (e.g. html module)'''
+
+ filename = xml_object.get('filename')
+ if filename is None:
+ definition_xml = copy.deepcopy(xml_object)
+ 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) and hasattr(
+ cls,
+ 'backcompat_paths'):
+ candidates = cls.backcompat_paths(filepath)
+ for candidate in candidates:
+ if system.resources_fs.exists(candidate):
+ filepath = candidate
+ break
+
+ try:
+ with system.resources_fs.open(filepath) as file:
+ definition_xml = cls.file_to_xml(file)
+ except Exception:
+ msg = 'Unable to load file contents at path %s for item %s' % (
+ filepath, location.url())
+ # Add info about where we are, but keep the traceback
+ raise Exception, msg, sys.exc_info()[2]
+
+ cls.clean_metadata_from_xml(definition_xml)
+ return cls.definition_from_xml(definition_xml, system)
+
+
@classmethod
def from_xml(cls, xml_data, system, org=None, course=None):
"""
@@ -180,7 +156,7 @@ class XmlDescriptor(XModuleDescriptor):
slug = xml_object.get('url_name', xml_object.get('slug'))
location = Location('i4x', org, course, xml_object.tag, slug)
- def metadata_loader():
+ def load_metadata():
metadata = {}
for attr in cls.metadata_attributes:
val = xml_object.get(attr)
@@ -192,49 +168,15 @@ class XmlDescriptor(XModuleDescriptor):
metadata[attr_map.metadata_key] = attr_map.to_metadata(val)
return metadata
- def definition_loader():
- filename = xml_object.get('filename')
- if filename is None:
- definition_xml = copy.deepcopy(xml_object)
- 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) and hasattr(cls, 'backcompat_paths'):
- candidates = cls.backcompat_paths(filepath)
- for candidate in candidates:
- if system.resources_fs.exists(candidate):
- filepath = candidate
- break
-
- try:
- with system.resources_fs.open(filepath) as file:
- definition_xml = cls.file_to_xml(file)
- except (ResourceNotFoundError, etree.XMLSyntaxError):
- msg = 'Unable to load file contents at path %s for item %s' % (filepath, location.url())
- log.exception(msg)
- system.error_handler(msg)
- # if error_handler didn't reraise, work around problem.
- error_elem = etree.Element('error')
- message_elem = etree.SubElement(error_elem, 'error_message')
- message_elem.text = msg
- stack_elem = etree.SubElement(error_elem, 'stack_trace')
- stack_elem.text = traceback.format_exc()
- return {'data': etree.tostring(error_elem)}
-
- cls.clean_metadata_from_xml(definition_xml)
- return cls.definition_from_xml(definition_xml, system)
-
+ definition = cls.load_definition(xml_object, system, location)
+ metadata = load_metadata()
+ # VS[compat] -- just have the url_name lookup once translation is done
+ slug = xml_object.get('url_name', xml_object.get('slug'))
return cls(
system,
- LazyLoadingDict(definition_loader),
+ definition,
location=location,
- metadata=LazyLoadingDict(metadata_loader),
+ metadata=metadata,
)
@classmethod
@@ -282,8 +224,8 @@ class XmlDescriptor(XModuleDescriptor):
# Write it to a file if necessary
if self.split_to_file(xml_object):
- # Put this object in it's own file
- filepath = self.__class__._format_filepath(self.category, self.name)
+ # Put this object in its own file
+ filepath = self.__class__._format_filepath(self.category, self.url_name)
resource_fs.makedir(os.path.dirname(filepath), allow_recreate=True)
with resource_fs.open(filepath, 'w') as file:
file.write(etree.tostring(xml_object, pretty_print=True))
@@ -296,10 +238,10 @@ class XmlDescriptor(XModuleDescriptor):
xml_object.tail = ''
- xml_object.set('filename', self.name)
+ xml_object.set('filename', self.url_name)
# Add the metadata
- xml_object.set('url_name', self.name)
+ xml_object.set('url_name', self.url_name)
for attr in self.metadata_attributes:
attr_map = self.xml_attribute_map.get(attr, AttrMap(attr))
metadata_key = attr_map.metadata_key
diff --git a/create-dev-env.sh b/create-dev-env.sh
index 91f9b81931..8a1a7e6f89 100755
--- a/create-dev-env.sh
+++ b/create-dev-env.sh
@@ -17,8 +17,10 @@ ouch() {
!! ERROR !!
The last command did not complete successfully,
- see $LOG for more details or trying running the
+ For more details or trying running the
script again with the -v flag.
+
+ Output of the script is recorded in $LOG
EOL
printf '\E[0m'
@@ -36,7 +38,7 @@ usage() {
Usage: $PROG [-c] [-v] [-h]
-c compile scipy and numpy
- -s do _not_ set --no-site-packages for virtualenv
+ -s give access to global site-packages for virtualenv
-v set -x + spew
-h this
@@ -61,28 +63,21 @@ clone_repos() {
if [[ -d "$BASE/mitx/.git" ]]; then
output "Pulling mitx"
cd "$BASE/mitx"
- git pull >>$LOG
+ git pull
else
output "Cloning mitx"
if [[ -d "$BASE/mitx" ]]; then
mv "$BASE/mitx" "${BASE}/mitx.bak.$$"
fi
- git clone git@github.com:MITx/mitx.git >>$LOG
+ git clone git@github.com:MITx/mitx.git
+ fi
+
+ if [[ ! -d "$BASE/mitx/askbot/.git" ]]; then
+ output "Cloning askbot as a submodule of mitx"
+ cd "$BASE/mitx"
+ git submodule update --init
fi
- cd "$BASE"
- if [[ -d "$BASE/askbot-devel/.git" ]]; then
- output "Pulling askbot-devel"
- cd "$BASE/askbot-devel"
- git pull >>$LOG
- else
- output "Cloning askbot-devel"
- if [[ -d "$BASE/askbot-devel" ]]; then
- mv "$BASE/askbot-devel" "${BASE}/askbot-devel.bak.$$"
- fi
- git clone git@github.com:MITx/askbot-devel >>$LOG
- fi
-
# By default, dev environments start with a copy of 6.002x
cd "$BASE"
mkdir -p "$BASE/data"
@@ -90,14 +85,14 @@ clone_repos() {
if [[ -d "$BASE/data/$REPO/.git" ]]; then
output "Pulling $REPO"
cd "$BASE/data/$REPO"
- git pull >>$LOG
+ git pull
else
output "Cloning $REPO"
if [[ -d "$BASE/data/$REPO" ]]; then
mv "$BASE/data/$REPO" "${BASE}/data/$REPO.bak.$$"
fi
cd "$BASE/data"
- git clone git@github.com:MITx/$REPO >>$LOG
+ git clone git@github.com:MITx/$REPO
fi
}
@@ -109,8 +104,8 @@ RUBY_VER="1.9.3"
NUMPY_VER="1.6.2"
SCIPY_VER="0.10.1"
BREW_FILE="$BASE/mitx/brew-formulas.txt"
-LOG="/var/tmp/install.log"
-APT_PKGS="curl git mercurial python-virtualenv build-essential python-dev gfortran liblapack-dev libfreetype6-dev libpng12-dev libxml2-dev libxslt-dev yui-compressor coffeescript"
+LOG="/var/tmp/install-$(date +%Y%m%d-%H%M%S).log"
+APT_PKGS="curl git python-virtualenv build-essential python-dev gfortran liblapack-dev libfreetype6-dev libpng12-dev libxml2-dev libxslt-dev yui-compressor coffeescript"
if [[ $EUID -eq 0 ]]; then
error "This script should not be run using sudo or as the root user"
@@ -163,23 +158,30 @@ cat<
+
+
+
+ © 2012 edX, some rights reserved.
+${chapter['name']}
+${chapter['display_name']}
-
% for section in chapter['sections']:
- ${msg}
+
${err}
+ % endfor
+ -
-
${ chapter['chapter'] }
+${ chapter['display_name'] }
-
%for section in chapter['sections']:
@@ -137,14 +137,14 @@ $(function() {
total = section['section_total'].possible
percentageString = "{0:.0%}".format( float(earned)/total) if earned > 0 and total > 0 else ""
%>
-
-
- ${ section['section'] } ${"({0:.3n}/{1:.3n}) {2}".format( float(earned), float(total), percentageString )}
+ ++ ${ section['display_name'] } ${"({0:.3n}/{1:.3n}) {2}".format( float(earned), float(total), percentageString )}
${section['format']} %if 'due' in section and section['due']!="": due ${section['due']} %endif - + %if len(section['scores']) > 0:-
${ "Problem Scores: " if section['graded'] else "Practice Scores: "}
@@ -153,7 +153,7 @@ $(function() {
%endfor
%endfor
- - Forum name: ${username} + Forum name: ${username}
- @@ -215,7 +215,7 @@ $(function() {
${section['name']}
+
+ ${section['display_name']}
${section['format']} ${"due " + section['due'] if 'due' in section and section['due'] != '' else ''}
diff --git a/lms/templates/courseware.html b/lms/templates/courseware.html
index c1658b3dee..a14f35d154 100644
--- a/lms/templates/courseware.html
+++ b/lms/templates/courseware.html
@@ -18,14 +18,6 @@
##
##
- ## image input: for clicking on images (see imageinput.html)
-
-
- ## TODO (cpennington): Remove this when we have a good way for modules to specify js to load on the page
- ## and in the wiki
-
-
-
<%static:js group='courseware'/>
<%include file="mathjax_include.html" />
@@ -43,7 +35,6 @@
diff --git a/lms/templates/dashboard.html b/lms/templates/dashboard.html
index ca3b273fc3..480568a5b9 100644
--- a/lms/templates/dashboard.html
+++ b/lms/templates/dashboard.html
@@ -34,10 +34,11 @@
Courseware Index
close
Course errors
+
+
+ % endif
+ % for (msg, err) in course_errors:
+
+
- % if user.is_authenticated():
-
-
+ % if user.is_authenticated():
+
Course Updates & News
+ ${get_course_info_section(course, 'updates')}
+ Course Handouts
+ ${get_course_info_section(course, 'handouts')}
+ Course Updates & News
+ ${get_course_info_section(course, 'guest_updates')}
+ Course Handouts
+ ${get_course_info_section(course, 'guest_handouts')}
+
+ login via openid +
diff --git a/lms/templates/module-error.html b/lms/templates/module-error.html
index 28597fa13c..7c731db17a 100644
--- a/lms/templates/module-error.html
+++ b/lms/templates/module-error.html
@@ -1,4 +1,13 @@
diff --git a/lms/templates/profile.html b/lms/templates/profile.html
index 6cda85fb03..8107bb1923 100644
--- a/lms/templates/profile.html
+++ b/lms/templates/profile.html
@@ -72,7 +72,7 @@ $(function() {
var new_email = $('#new_email_field').val();
var new_password = $('#new_email_password').val();
- postJSON('/change_email',{"new_email":new_email,
+ postJSON('/change_email',{"new_email":new_email,
"password":new_password},
function(data){
if(data.success){
@@ -81,7 +81,7 @@ $(function() {
$("#change_email_error").html(data.error);
}
});
- log_event("profile", {"type":"email_change_request",
+ log_event("profile", {"type":"email_change_request",
"old_email":"${email}",
"new_email":new_email});
return false;
@@ -91,7 +91,7 @@ $(function() {
var new_name = $('#new_name_field').val();
var rationale = $('#name_rationale_field').val();
- postJSON('/change_name',{"new_name":new_name,
+ postJSON('/change_name',{"new_name":new_name,
"rationale":rationale},
function(data){
if(data.success){
@@ -100,7 +100,7 @@ $(function() {
$("#change_name_error").html(data.error);
}
});
- log_event("profile", {"type":"name_change_request",
+ log_event("profile", {"type":"name_change_request",
"new_name":new_name,
"rationale":rationale});
return false;
@@ -125,9 +125,9 @@ $(function() {
There has been an error on the MITx servers
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.
+ +% if is_staff: +Staff-only details below:
+ +Error: ${error | h}
+ +Raw data: ${data | h}
+% endif +-
%for chapter in courseware_summary:
- %if not chapter['chapter'] == "hidden":
+ %if not chapter['display_name'] == "hidden":
-
+ % endif
diff --git a/lms/templates/staticbook.html b/lms/templates/staticbook.html
index f5b184cc1c..eae70cdd84 100644
--- a/lms/templates/staticbook.html
+++ b/lms/templates/staticbook.html
@@ -67,7 +67,6 @@ $("#open_close_accordion a").click(function(){
Change e-mail
+Change e-mail
+ % if has_extauth_info is UNDEFINED:Already have an account? Login.