Skip to content

E
edx-platform
Commit 0c9e5825 by Victor Shnayder
Options

Merge pull request #336 from MITx/feature/victor/xml-verify 

Feature/victor/xml verify
parents 377fbf8c f0e92fcc
Showing with 183 additions and 292 deletions
common/djangoapps/xmodule_modifiers.py
......@@ -89,8 +89,8 @@ def add_histogram(get_html, module):
else:
edit_link = False
staff_context = {'definition': dict(module.definition),
'metadata': dict(module.metadata),
staff_context = {'definition': json.dumps(module.definition, indent=4),
'metadata': json.dumps(module.metadata, indent=4),
'element_id': module.location.html_id(),
'edit_link': edit_link,
'histogram': json.dumps(histogram),
......
common/lib/supertrace.py 0 → 100644
"""
A handy util to print a django-debug-screen-like stack trace with
values of local variables.
"""
import sys, traceback
from django.utils.encoding import smart_unicode
def supertrace(max_len=160):
"""
Print the usual traceback information, followed by a listing of all the
local variables in each frame. Should be called from an exception handler.
if max_len is not None, will print up to max_len chars for each local variable.
(cite: modified from somewhere on stackoverflow)
"""
tb = sys.exc_info()[2]
while True:
if not tb.tb_next:
break
tb = tb.tb_next
stack = []
frame = tb.tb_frame
while frame:
stack.append(f)
frame = frame.f_back
stack.reverse()
# First print the regular traceback
traceback.print_exc()
print "Locals by frame, innermost last"
for frame in stack:
print
print "Frame %s in %s at line %s" % (frame.f_code.co_name,
frame.f_code.co_filename,
frame.f_lineno)
for key, value in frame.f_locals.items():
print ("\t%20s = " % smart_unicode(key, errors='ignore')),
# We have to be careful not to cause a new error in our error
# printer! Calling str() on an unknown object could cause an
# error.
try:
s = smart_unicode(value, errors='ignore')
if max_len is not None:
s = s[:max_len]
print s
except:
print "<ERROR WHILE PRINTING VALUE>"
common/lib/xmodule/lazy_dict.py deleted 100644 → 0
from collections import MutableMapping
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
common/lib/xmodule/progress.py deleted 100644 → 0
'''
Progress class for modules. Represents where a student is in a module.
Useful things to know:
- Use Progress.to_js_status_str() to convert a progress into a simple
status string to pass to js.
- Use Progress.to_js_detail_str() to convert a progress into a more detailed
string to pass to js.
In particular, these functions have a canonical handing of None.
For most subclassing needs, you should only need to reimplement
frac() and __str__().
'''
from collections import namedtuple
import numbers
class Progress(object):
'''Represents a progress of a/b (a out of b done)
a and b must be numeric, but not necessarily integer, with
0 <= a <= b and b > 0.
Progress can only represent Progress for modules where that makes sense. Other
modules (e.g. html) should return None from get_progress().
TODO: add tag for module type? Would allow for smarter merging.
'''
def __init__(self, a, b):
'''Construct a Progress object. a and b must be numbers, and must have
0 <= a <= b and b > 0
'''
# Want to do all checking at construction time, so explicitly check types
if not (isinstance(a, numbers.Number) and
isinstance(b, numbers.Number)):
raise TypeError('a and b must be numbers. Passed {0}/{1}'.format(a, b))
if not (0 <= a <= b and b > 0):
raise ValueError(
'fraction a/b = {0}/{1} must have 0 <= a <= b and b > 0'.format(a, b))
self._a = a
self._b = b
def frac(self):
''' Return tuple (a,b) representing progress of a/b'''
return (self._a, self._b)
def percent(self):
''' Returns a percentage progress as a float between 0 and 100.
subclassing note: implemented in terms of frac(), assumes sanity
checking is done at construction time.
'''
(a, b) = self.frac()
return 100.0 * a / b
def started(self):
''' Returns True if fractional progress is greater than 0.
subclassing note: implemented in terms of frac(), assumes sanity
checking is done at construction time.
'''
return self.frac()[0] > 0
def inprogress(self):
''' Returns True if fractional progress is strictly between 0 and 1.
subclassing note: implemented in terms of frac(), assumes sanity
checking is done at construction time.
'''
(a, b) = self.frac()
return a > 0 and a < b
def done(self):
''' Return True if this represents done.
subclassing note: implemented in terms of frac(), assumes sanity
checking is done at construction time.
'''
(a, b) = self.frac()
return a == b
def ternary_str(self):
''' Return a string version of this progress: either
"none", "in_progress", or "done".
subclassing note: implemented in terms of frac()
'''
(a, b) = self.frac()
if a == 0:
return "none"
if a < b:
return "in_progress"
return "done"
def __eq__(self, other):
''' Two Progress objects are equal if they have identical values.
Implemented in terms of frac()'''
if not isinstance(other, Progress):
return False
(a, b) = self.frac()
(a2, b2) = other.frac()
return a == a2 and b == b2
def __ne__(self, other):
''' The opposite of equal'''
return not self.__eq__(other)
def __str__(self):
''' Return a string representation of this string.
subclassing note: implemented in terms of frac().
'''
(a, b) = self.frac()
return "{0}/{1}".format(a, b)
@staticmethod
def add_counts(a, b):
'''Add two progress indicators, assuming that each represents items done:
(a / b) + (c / d) = (a + c) / (b + d).
If either is None, returns the other.
'''
if a is None:
return b
if b is None:
return a
# get numerators + denominators
(n, d) = a.frac()
(n2, d2) = b.frac()
return Progress(n + n2, d + d2)
@staticmethod
def to_js_status_str(progress):
'''
Return the "status string" version of the passed Progress
object that should be passed to js. Use this function when
sending Progress objects to js to limit dependencies.
'''
if progress is None:
return "NA"
return progress.ternary_str()
@staticmethod
def to_js_detail_str(progress):
'''
Return the "detail string" version of the passed Progress
object that should be passed to js. Use this function when
passing Progress objects to js to limit dependencies.
'''
if progress is None:
return "NA"
return str(progress)
common/lib/xmodule/xmodule/editing_module.py
......@@ -20,5 +20,10 @@ class EditingDescriptor(MakoModuleDescriptor):
def get_context(self):
return {
'module': self,
'data': self.definition['data'],
'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,
}
common/lib/xmodule/xmodule/error_module.py
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
import logging
log = logging.getLogger(__name__)
......@@ -14,14 +17,11 @@ class ErrorModule(XModule):
'''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')
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):
......@@ -31,29 +31,36 @@ class ErrorDescriptor(EditingDescriptor):
module_class = ErrorModule
@classmethod
def from_xml(cls, xml_data, system, org=None, course=None, err=None):
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.
issue. (should be a string, or convert usefully into one).
'''
definition = {}
if err is not None:
definition['error'] = err
# 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
except etree.XMLSyntaxError as err:
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
definition['error'] = err
inner['error_msg'] = exc_info_to_str(sys.exc_info())
definition['data'] = xml_data
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']
......@@ -71,10 +78,12 @@ class ErrorDescriptor(EditingDescriptor):
files, etc. That would just get re-wrapped on import.
'''
try:
xml = etree.fromstring(self.definition['data'])
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']
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)
common/lib/xmodule/xmodule/errortracker.py
......@@ -8,6 +8,12 @@ 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)
......@@ -27,7 +33,7 @@ def make_error_tracker():
'''Log errors'''
exc_str = ''
if in_exception_handler():
exc_str = ''.join(traceback.format_exception(*sys.exc_info()))
exc_str = exc_info_to_str(sys.exc_info())
errors.append((msg, exc_str))
......
common/lib/xmodule/xmodule/html_module.py
......@@ -5,11 +5,11 @@ import os
import sys
from lxml import etree
from xmodule.x_module import XModule
from xmodule.xml_module import XmlDescriptor
from xmodule.editing_module import EditingDescriptor
from stringify import stringify_children
from html_checker import check_html
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")
......
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 logging
import re
from collections import namedtuple
from .exceptions import InvalidLocationError, InsufficientSpecificationError
import logging
from xmodule.errortracker import ErrorLog, make_error_tracker
log = logging.getLogger('mitx.' + 'modulestore')
......@@ -290,3 +293,38 @@ class ModuleStore(object):
'''
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
common/lib/xmodule/xmodule/modulestore/mongo.py
......@@ -11,7 +11,7 @@ from xmodule.x_module import XModuleDescriptor
from xmodule.mako_module import MakoDescriptorSystem
from mitxmako.shortcuts import render_to_string
from . import ModuleStore, Location
from . import ModuleStoreBase, Location
from .exceptions import (ItemNotFoundError,
NoPathToItem, DuplicateItemError)
......@@ -38,7 +38,7 @@ class CachingDescriptorSystem(MakoDescriptorSystem):
resources_fs: a filesystem, as per MakoDescriptorSystem
error_tracker:
error_tracker: a function that logs errors for later display to users
render_template: a function for rendering templates, as per
MakoDescriptorSystem
......@@ -73,7 +73,7 @@ def location_to_query(location):
return query
class MongoModuleStore(ModuleStore):
class MongoModuleStore(ModuleStoreBase):
"""
A Mongodb backed ModuleStore
"""
......@@ -81,6 +81,9 @@ class MongoModuleStore(ModuleStore):
# TODO (cpennington): Enable non-filesystem filestores
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
......
common/lib/xmodule/xmodule/modulestore/xml.py
......@@ -12,7 +12,7 @@ from xmodule.course_module import CourseDescriptor
from xmodule.mako_module import MakoDescriptorSystem
from cStringIO import StringIO
from . import ModuleStore, Location
from . import ModuleStoreBase, Location
from .exceptions import ItemNotFoundError
etree.set_default_parser(
......@@ -98,7 +98,7 @@ class ImportSystem(XMLParsingSystem, MakoDescriptorSystem):
error_tracker, process_xml, **kwargs)
class XMLModuleStore(ModuleStore):
class XMLModuleStore(ModuleStoreBase):
"""
An XML backed ModuleStore
"""
......@@ -118,13 +118,12 @@ class XMLModuleStore(ModuleStore):
course_dirs: If specified, the list of course_dirs to load. Otherwise,
load all course dirs
"""
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.location_errors = {} # location -> ErrorLog
if default_class is None:
self.default_class = None
......@@ -148,12 +147,14 @@ class XMLModuleStore(ModuleStore):
for course_dir in course_dirs:
try:
# make a tracker, then stick in the right place once the course loads
# and we know its location
# 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
self._location_errors[course_descriptor.location] = errorlog
except:
msg = "Failed to load course '%s'" % course_dir
log.exception(msg)
......@@ -221,23 +222,6 @@ class XMLModuleStore(ModuleStore):
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. If there were errors on loading,
......@@ -245,9 +229,11 @@ class XMLModuleStore(ModuleStore):
"""
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
......@@ -258,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
......@@ -268,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
......
common/lib/xmodule/tests/test_import.py → common/lib/xmodule/xmodule/tests/test_import.py
......@@ -73,6 +73,7 @@ class ImportTestCase(unittest.TestCase):
def test_reimport(self):
'''Make sure an already-exported error xml tag loads properly'''
self.maxDiff = None
bad_xml = '''<sequential display_name="oops"><video url="hi"></sequential>'''
system = self.get_system()
descriptor = XModuleDescriptor.load_from_xml(bad_xml, system, 'org', 'course',
......
common/lib/xmodule/tests/test_stringify.py → common/lib/xmodule/xmodule/tests/test_stringify.py
from nose.tools import assert_equals
from lxml import etree
from stringify import stringify_children
from xmodule.stringify import stringify_children
def test_stringify():
html = '''<html a="b" foo="bar">Hi <div x="foo">there <span>Bruce</span><b>!</b></div></html>'''
text = 'Hi <div x="foo">there <span>Bruce</span><b>!</b></div>'
html = '''<html a="b" foo="bar">{0}</html>'''.format(text)
xml = etree.fromstring(html)
out = stringify_children(xml)
assert_equals(out, '''Hi <div x="foo">there <span>Bruce</span><b>!</b></div>''')
assert_equals(out, text)
common/lib/xmodule/xmodule/x_module.py
from lxml import etree
from lxml.etree import XMLSyntaxError
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 xmodule.errortracker import exc_info_to_str
log = logging.getLogger('mitx.' + __name__)
......@@ -471,7 +473,9 @@ class XModuleDescriptor(Plugin, HTMLSnippet):
msg = "Error loading from xml."
log.exception(msg)
system.error_tracker(msg)
descriptor = ErrorDescriptor.from_xml(xml_data, system, org, course, err)
err_msg = msg + "\n" + exc_info_to_str(sys.exc_info())
descriptor = ErrorDescriptor.from_xml(xml_data, system, org, course,
err_msg)
return descriptor
......
lms/djangoapps/courseware/views.py
......@@ -117,7 +117,7 @@ def profile(request, course_id, student_id=None):
student_module_cache = StudentModuleCache(request.user, course)
course_module, _, _, _ = get_module(request.user, request, course.location, student_module_cache)
context = {'name': user_info.name,
'username': student.username,
'location': user_info.location,
......@@ -243,6 +243,7 @@ def index(request, course_id, chapter=None, section=None,
return result
@ensure_csrf_cookie
def jump_to(request, location):
'''
......@@ -269,7 +270,7 @@ def jump_to(request, location):
except NoPathToItem:
raise Http404("This location is not in any class: {0}".format(location))
# Rely on index to do all error handling
return index(request, course_id, chapter, section, position)
@ensure_csrf_cookie
......
lms/templates/module-error-staff.html deleted 100644 → 0
<section class="outside-app">
<h1>There has been an error on the <em>MITx</em> servers</h1>
<p>We're sorry, this module is temporarily unavailable. Our staff is working to fix it as soon as possible. Please email us at <a href="mailto:technical@mitx.mit.edu">technical@mitx.mit.edu</a> to report any problems or downtime.</p>
<h1>Staff-only details below:</h1>
<p>Error: ${error}</p>
<p>Raw data: ${data}</p>
</section>
lms/templates/module-error.html
<section class="outside-app">
<h1>There has been an error on the <em>MITx</em> servers</h1>
<p>We're sorry, this module is temporarily unavailable. Our staff is working to fix it as soon as possible. Please email us at <a href="mailto:technical@mitx.mit.edu">technical@mitx.mit.edu</a> to report any problems or downtime.</p>
% if is_staff:
<h1>Staff-only details below:</h1>
<p>Error: ${error | h}</p>
<p>Raw data: ${data | h}</p>
% endif
</section>
Markdown is supported
0% or
You are about to add 0 people to the discussion. Proceed with caution.
Finish editing this message first!
Please register or to comment