${section['format']}
%if 'due' in section and section['due']!="":
From 4e6a9b0df7e3cffc8e63590b8124c69dd2369b34 Mon Sep 17 00:00:00 2001
From: Victor Shnayder
Date: Mon, 30 Jul 2012 10:36:33 -0400
Subject: [PATCH 030/124] Add kwargs back to DescriptorSystem init()s
* allow future expansion without breaking interface.
---
common/lib/xmodule/xmodule/mako_module.py | 4 ++--
common/lib/xmodule/xmodule/modulestore/xml.py | 6 +++---
common/lib/xmodule/xmodule/x_module.py | 7 ++++---
3 files changed, 9 insertions(+), 8 deletions(-)
diff --git a/common/lib/xmodule/xmodule/mako_module.py b/common/lib/xmodule/xmodule/mako_module.py
index fcc47aaaaf..213e9077db 100644
--- a/common/lib/xmodule/xmodule/mako_module.py
+++ b/common/lib/xmodule/xmodule/mako_module.py
@@ -3,9 +3,9 @@ from x_module import XModuleDescriptor, DescriptorSystem
class MakoDescriptorSystem(DescriptorSystem):
def __init__(self, load_item, resources_fs, error_handler,
- render_template):
+ render_template, **kwargs):
super(MakoDescriptorSystem, self).__init__(
- load_item, resources_fs, error_handler)
+ load_item, resources_fs, error_handler, **kwargs)
self.render_template = render_template
diff --git a/common/lib/xmodule/xmodule/modulestore/xml.py b/common/lib/xmodule/xmodule/modulestore/xml.py
index 7dd6868f78..8a084b19ee 100644
--- a/common/lib/xmodule/xmodule/modulestore/xml.py
+++ b/common/lib/xmodule/xmodule/modulestore/xml.py
@@ -29,7 +29,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_handler, **kwargs):
"""
A class that handles loading from xml. Does some munging to ensure that
all elements have unique slugs.
@@ -88,9 +88,9 @@ class ImportSystem(XMLParsingSystem, MakoDescriptorSystem):
resources_fs = OSFS(xmlstore.data_dir / course_dir)
MakoDescriptorSystem.__init__(self, load_item, resources_fs,
- error_handler, render_template)
+ error_handler, render_template, **kwargs)
XMLParsingSystem.__init__(self, load_item, resources_fs,
- error_handler, process_xml)
+ error_handler, process_xml, **kwargs)
diff --git a/common/lib/xmodule/xmodule/x_module.py b/common/lib/xmodule/xmodule/x_module.py
index 7bb98dcdc5..97ae307809 100644
--- a/common/lib/xmodule/xmodule/x_module.py
+++ b/common/lib/xmodule/xmodule/x_module.py
@@ -521,7 +521,7 @@ class XModuleDescriptor(Plugin, HTMLSnippet):
class DescriptorSystem(object):
- def __init__(self, load_item, resources_fs, error_handler):
+ def __init__(self, load_item, resources_fs, error_handler, **kwargs):
"""
load_item: Takes a Location and returns an XModuleDescriptor
@@ -561,14 +561,15 @@ class DescriptorSystem(object):
class XMLParsingSystem(DescriptorSystem):
- def __init__(self, load_item, resources_fs, error_handler, process_xml):
+ def __init__(self, load_item, resources_fs, error_handler, process_xml, **kwargs):
"""
load_item, resources_fs, error_handler: 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_handler,
+ **kwargs)
self.process_xml = process_xml
From ef6da22ac35b70f313ee8f9e60c89d68e89bd745 Mon Sep 17 00:00:00 2001
From: Victor Shnayder
Date: Mon, 30 Jul 2012 10:42:23 -0400
Subject: [PATCH 031/124] Add a fallback MalformedDescriptor
* when things don't load normally, use this
* separate raw editing functionality into EditingDescriptor
* raw descriptor just enforces that xml is valid
* add a MalformedDescriptor that just saves a string
* Fallback to it on import.
---
common/lib/xmodule/tests/test_import.py | 38 +++++++++++++++++
common/lib/xmodule/xmodule/editing_module.py | 24 +++++++++++
.../lib/xmodule/xmodule/malformed_module.py | 41 +++++++++++++++++++
common/lib/xmodule/xmodule/raw_module.py | 18 ++------
common/lib/xmodule/xmodule/x_module.py | 34 ++++++++++-----
5 files changed, 131 insertions(+), 24 deletions(-)
create mode 100644 common/lib/xmodule/tests/test_import.py
create mode 100644 common/lib/xmodule/xmodule/editing_module.py
create mode 100644 common/lib/xmodule/xmodule/malformed_module.py
diff --git a/common/lib/xmodule/tests/test_import.py b/common/lib/xmodule/tests/test_import.py
new file mode 100644
index 0000000000..c0bd9af3d0
--- /dev/null
+++ b/common/lib/xmodule/tests/test_import.py
@@ -0,0 +1,38 @@
+from path import path
+
+import unittest
+
+from xmodule.x_module import XMLParsingSystem, XModuleDescriptor
+from xmodule.errorhandlers import ignore_errors_handler
+from xmodule.modulestore import Location
+
+class ImportTestCase(unittest.TestCase):
+ '''Make sure module imports work properly, including for malformed inputs'''
+
+ def test_fallback(self):
+ '''Make sure that malformed xml loads as a MalformedDescriptorb.'''
+
+ bad_xml = ''''''
+
+ # Shouldn't need any system params, because the initial parse should fail
+ def load_item(loc):
+ raise Exception("Shouldn't be called")
+
+ resources_fs = None
+
+ def process_xml(xml):
+ raise Exception("Shouldn't be called")
+
+
+ def render_template(template, context):
+ raise Exception("Shouldn't be called")
+
+ system = XMLParsingSystem(load_item, resources_fs,
+ ignore_errors_handler, process_xml)
+ system.render_template = render_template
+
+ descriptor = XModuleDescriptor.load_from_xml(bad_xml, system, 'org', 'course',
+ None)
+
+ self.assertEqual(descriptor.__class__.__name__,
+ 'MalformedDescriptor')
diff --git a/common/lib/xmodule/xmodule/editing_module.py b/common/lib/xmodule/xmodule/editing_module.py
new file mode 100644
index 0000000000..4188165a24
--- /dev/null
+++ b/common/lib/xmodule/xmodule/editing_module.py
@@ -0,0 +1,24 @@
+from pkg_resources import resource_string
+from lxml import etree
+from xmodule.mako_module import MakoModuleDescriptor
+import logging
+
+log = logging.getLogger(__name__)
+
+class EditingDescriptor(MakoModuleDescriptor):
+ """
+ Module that provides a raw editing view of its data and children. It does not
+ perform any validation on its definition---just passes it along to the browser.
+
+ This class is intended to be used as a mixin.
+ """
+ mako_template = "widgets/raw-edit.html"
+
+ js = {'coffee': [resource_string(__name__, 'js/src/raw/edit.coffee')]}
+ js_module_name = "RawDescriptor"
+
+ def get_context(self):
+ return {
+ 'module': self,
+ 'data': self.definition['data'],
+ }
diff --git a/common/lib/xmodule/xmodule/malformed_module.py b/common/lib/xmodule/xmodule/malformed_module.py
new file mode 100644
index 0000000000..803813dee8
--- /dev/null
+++ b/common/lib/xmodule/xmodule/malformed_module.py
@@ -0,0 +1,41 @@
+from pkg_resources import resource_string
+from lxml import etree
+from xmodule.mako_module import MakoModuleDescriptor
+from xmodule.xml_module import XmlDescriptor
+from xmodule.editing_module import EditingDescriptor
+
+import logging
+
+log = logging.getLogger(__name__)
+
+class MalformedDescriptor(EditingDescriptor):
+ """
+ Module that provides a raw editing view of broken xml.
+ """
+
+ @classmethod
+ def from_xml(cls, xml_data, system, org=None, course=None):
+ '''Create an instance of this descriptor from the supplied data.
+
+ Does not try to parse the data--just stores it.
+ '''
+
+ # TODO (vshnayder): how does one get back from this to a valid descriptor?
+ # try to parse and if successfull, send back to x_module?
+
+ definition = { 'data' : xml_data }
+ # TODO (vshnayder): Do we need a valid slug here? Just pick a random
+ # 64-bit num?
+ location = ['i4x', org, course, 'malformed', 'slug']
+ metadata = {} # stays in the xml_data
+
+ return cls(system, definition, location=location, metadata=metadata)
+
+ def export_to_xml(self, resource_fs):
+ '''
+ Export as a string wrapped in xml
+ '''
+ root = etree.Element('malformed')
+ root.text = self.definition['data']
+ return etree.tostring(root)
+
diff --git a/common/lib/xmodule/xmodule/raw_module.py b/common/lib/xmodule/xmodule/raw_module.py
index 90f4139bd5..f9f358f945 100644
--- a/common/lib/xmodule/xmodule/raw_module.py
+++ b/common/lib/xmodule/xmodule/raw_module.py
@@ -1,27 +1,17 @@
from pkg_resources import resource_string
from lxml import etree
+from xmodule.editing_module import EditingDescriptor
from xmodule.mako_module import MakoModuleDescriptor
from xmodule.xml_module import XmlDescriptor
import logging
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)}
diff --git a/common/lib/xmodule/xmodule/x_module.py b/common/lib/xmodule/xmodule/x_module.py
index 97ae307809..c4c5110abc 100644
--- a/common/lib/xmodule/xmodule/x_module.py
+++ b/common/lib/xmodule/xmodule/x_module.py
@@ -1,10 +1,12 @@
from lxml import etree
+from lxml.etree import XMLSyntaxError
import pkg_resources
import logging
+from fs.errors import ResourceNotFoundError
+from functools import partial
from xmodule.modulestore import Location
-from functools import partial
log = logging.getLogger('mitx.' + __name__)
@@ -443,16 +445,28 @@ 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 (ResourceNotFoundError, XMLSyntaxError) as err:
+ # Didn't load properly. Fall back on loading as a malformed
+ # descriptor. This should never error due to formatting.
+
+ # Put import here to avoid circular import errors
+ from xmodule.malformed_module import MalformedDescriptor
+
+ descriptor = MalformedDescriptor.from_xml(xml_data, system, org, course)
+
+ return descriptor
@classmethod
def from_xml(cls, xml_data, system, org=None, course=None):
From 53608922ba2cf74f27e5f05a23694943593704f4 Mon Sep 17 00:00:00 2001
From: Victor Shnayder
Date: Mon, 30 Jul 2012 11:36:43 -0400
Subject: [PATCH 032/124] Make the malformed descriptor import properly
* Also get rid of lazy loading of metadata and definition
---
common/lib/xmodule/setup.py | 1 +
.../lib/xmodule/xmodule/malformed_module.py | 11 +-
common/lib/xmodule/xmodule/x_module.py | 1 +
common/lib/xmodule/xmodule/xml_module.py | 101 +++++++++---------
4 files changed, 62 insertions(+), 52 deletions(-)
diff --git a/common/lib/xmodule/setup.py b/common/lib/xmodule/setup.py
index b495cd0aee..09a6b7e712 100644
--- a/common/lib/xmodule/setup.py
+++ b/common/lib/xmodule/setup.py
@@ -25,6 +25,7 @@ setup(
"discuss = xmodule.backcompat_module:TranslateCustomTagDescriptor",
"html = xmodule.html_module:HtmlDescriptor",
"image = xmodule.backcompat_module:TranslateCustomTagDescriptor",
+ "malformed = xmodule.malformed_module:MalformedDescriptor",
"problem = xmodule.capa_module:CapaDescriptor",
"problemset = xmodule.vertical_module:VerticalDescriptor",
"section = xmodule.backcompat_module:SemanticSectionDescriptor",
diff --git a/common/lib/xmodule/xmodule/malformed_module.py b/common/lib/xmodule/xmodule/malformed_module.py
index 803813dee8..54032cf7d2 100644
--- a/common/lib/xmodule/xmodule/malformed_module.py
+++ b/common/lib/xmodule/xmodule/malformed_module.py
@@ -20,8 +20,15 @@ class MalformedDescriptor(EditingDescriptor):
Does not try to parse the data--just stores it.
'''
- # TODO (vshnayder): how does one get back from this to a valid descriptor?
- # try to parse and if successfull, send back to x_module?
+ #log.debug("processing '{0}'".format(xml_data))
+ try:
+ xml_obj = etree.fromstring(xml_data)
+ if xml_obj.tag == 'malformed':
+ xml_data = xml_obj.text
+ # TODO (vshnayder): how does one get back from this to a valid descriptor?
+ # For now, have to fix manually.
+ except etree.XMLSyntaxError:
+ pass
definition = { 'data' : xml_data }
# TODO (vshnayder): Do we need a valid slug here? Just pick a random
diff --git a/common/lib/xmodule/xmodule/x_module.py b/common/lib/xmodule/xmodule/x_module.py
index c4c5110abc..8803c37d78 100644
--- a/common/lib/xmodule/xmodule/x_module.py
+++ b/common/lib/xmodule/xmodule/x_module.py
@@ -464,6 +464,7 @@ class XModuleDescriptor(Plugin, HTMLSnippet):
# Put import here to avoid circular import errors
from xmodule.malformed_module import MalformedDescriptor
+ #system.error_handler("Error loading from xml.")
descriptor = MalformedDescriptor.from_xml(xml_data, system, org, course)
return descriptor
diff --git a/common/lib/xmodule/xmodule/xml_module.py b/common/lib/xmodule/xmodule/xml_module.py
index 6750906eb4..5786dbd227 100644
--- a/common/lib/xmodule/xmodule/xml_module.py
+++ b/common/lib/xmodule/xmodule/xml_module.py
@@ -11,66 +11,65 @@ import os
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
+# # 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).
+# """
-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 __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()
- def __getitem__(self, name):
- if not (self._loaded or name in self._contents or name in self._deleted):
- self.load()
+# return self._contents[name]
- return self._contents[name]
+# def __setitem__(self, name, value):
+# self._contents[name] = value
+# self._deleted.discard(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 __delitem__(self, name):
- del self._contents[name]
- self._deleted.add(name)
+# def __contains__(self, name):
+# self.load()
+# return name in self._contents
- def __contains__(self, name):
- self.load()
- return name in self._contents
+# def __len__(self):
+# self.load()
+# return len(self._contents)
- def __len__(self):
- self.load()
- return len(self._contents)
+# def __iter__(self):
+# self.load()
+# return iter(self._contents)
- def __iter__(self):
- self.load()
- return iter(self._contents)
+# def __repr__(self):
+# self.load()
+# return repr(self._contents)
- def __repr__(self):
- self.load()
- return repr(self._contents)
+# def load(self):
+# if self._loaded:
+# return
- def load(self):
- if self._loaded:
- return
-
- loaded_contents = self._loader()
- loaded_contents.update(self._contents)
- self._contents = loaded_contents
- self._loaded = True
+# 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')
@@ -230,11 +229,13 @@ class XmlDescriptor(XModuleDescriptor):
cls.clean_metadata_from_xml(definition_xml)
return cls.definition_from_xml(definition_xml, system)
+ # 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
From 0d83d2e645a4a701ebad7e5158493c198d552370 Mon Sep 17 00:00:00 2001
From: Victor Shnayder
Date: Mon, 30 Jul 2012 11:51:14 -0400
Subject: [PATCH 033/124] Add roundtrip test for malformed module
* also fix error message in backcompat_module
---
common/lib/xmodule/tests/test_import.py | 35 ++++++++++++++++---
.../lib/xmodule/xmodule/backcompat_module.py | 2 +-
2 files changed, 31 insertions(+), 6 deletions(-)
diff --git a/common/lib/xmodule/tests/test_import.py b/common/lib/xmodule/tests/test_import.py
index c0bd9af3d0..6643b93a67 100644
--- a/common/lib/xmodule/tests/test_import.py
+++ b/common/lib/xmodule/tests/test_import.py
@@ -9,11 +9,9 @@ from xmodule.modulestore import Location
class ImportTestCase(unittest.TestCase):
'''Make sure module imports work properly, including for malformed inputs'''
- def test_fallback(self):
- '''Make sure that malformed xml loads as a MalformedDescriptorb.'''
-
- bad_xml = ''''''
-
+ @staticmethod
+ def get_system():
+ '''Get a dummy system'''
# Shouldn't need any system params, because the initial parse should fail
def load_item(loc):
raise Exception("Shouldn't be called")
@@ -31,8 +29,35 @@ class ImportTestCase(unittest.TestCase):
ignore_errors_handler, process_xml)
system.render_template = render_template
+ return system
+
+ def test_fallback(self):
+ '''Make sure that malformed xml loads as a MalformedDescriptorb.'''
+
+ bad_xml = ''''''
+
+ system = self.get_system()
+
descriptor = XModuleDescriptor.load_from_xml(bad_xml, system, 'org', 'course',
None)
self.assertEqual(descriptor.__class__.__name__,
'MalformedDescriptor')
+
+ def test_reimport(self):
+ '''Make sure an already-exported malformed xml tag loads properly'''
+
+ bad_xml = ''''''
+ system = self.get_system()
+ descriptor = XModuleDescriptor.load_from_xml(bad_xml, system, 'org', 'course',
+ None)
+ resource_fs = None
+ tag_xml = descriptor.export_to_xml(resource_fs)
+ re_import_descriptor = XModuleDescriptor.load_from_xml(tag_xml, system,
+ 'org', 'course',
+ None)
+ self.assertEqual(re_import_descriptor.__class__.__name__,
+ 'MalformedDescriptor')
+
+ self.assertEqual(descriptor.definition['data'],
+ re_import_descriptor.definition['data'])
diff --git a/common/lib/xmodule/xmodule/backcompat_module.py b/common/lib/xmodule/xmodule/backcompat_module.py
index 997ad476c4..da0d6788e4 100644
--- a/common/lib/xmodule/xmodule/backcompat_module.py
+++ b/common/lib/xmodule/xmodule/backcompat_module.py
@@ -32,7 +32,7 @@ def process_includes(fn):
# read in and convert to XML
incxml = etree.XML(ifp.read())
- # insert new XML into tree in place of inlcude
+ # insert new XML into tree in place of include
parent.insert(parent.index(next_include), incxml)
except Exception:
msg = "Error in problem xml include: %s" % (etree.tostring(next_include, pretty_print=True))
From 10054e95ce0e0962e975ff236b5237f659cd6bc7 Mon Sep 17 00:00:00 2001
From: Victor Shnayder
Date: Mon, 30 Jul 2012 15:09:33 -0400
Subject: [PATCH 034/124] add message to import command
---
cms/djangoapps/contentstore/management/commands/import.py | 3 +++
1 file changed, 3 insertions(+)
diff --git a/cms/djangoapps/contentstore/management/commands/import.py b/cms/djangoapps/contentstore/management/commands/import.py
index 69aaa35a7d..a6790ad59b 100644
--- a/cms/djangoapps/contentstore/management/commands/import.py
+++ b/cms/djangoapps/contentstore/management/commands/import.py
@@ -23,4 +23,7 @@ class Command(BaseCommand):
course_dirs = args[1:]
else:
course_dirs = None
+ print "Importing. Data_dir={data}, course_dirs={courses}".format(
+ data=data_dir,
+ courses=course_dirs)
import_from_xml(modulestore(), data_dir, course_dirs)
From c53ed6a23830f842aa130c104e7c9125509c4b9a Mon Sep 17 00:00:00 2001
From: Victor Shnayder
Date: Mon, 30 Jul 2012 15:10:21 -0400
Subject: [PATCH 035/124] remove some debugging messages
---
common/lib/capa/capa/capa_problem.py | 2 +-
1 file changed, 1 insertion(+), 1 deletion(-)
diff --git a/common/lib/capa/capa/capa_problem.py b/common/lib/capa/capa/capa_problem.py
index ed99c71635..93b6a085c1 100644
--- a/common/lib/capa/capa/capa_problem.py
+++ b/common/lib/capa/capa/capa_problem.py
@@ -329,7 +329,7 @@ class LoncapaProblem(object):
# path is an absolute path or a path relative to the data dir
dir = os.path.join(self.system.filestore.root_path, dir)
abs_dir = os.path.normpath(dir)
- log.debug("appending to path: %s" % abs_dir)
+ #log.debug("appending to path: %s" % abs_dir)
path.append(abs_dir)
return path
From d750d945fdd855ea05e69d346b248b222c6e4136 Mon Sep 17 00:00:00 2001
From: Victor Shnayder
Date: Mon, 30 Jul 2012 15:11:38 -0400
Subject: [PATCH 036/124] Remove malformed tags when contents aren't malformed
anymore.
---
common/lib/xmodule/tests/test_import.py | 26 +++++++++++++++-
.../lib/xmodule/xmodule/malformed_module.py | 30 +++++++++++++++----
2 files changed, 49 insertions(+), 7 deletions(-)
diff --git a/common/lib/xmodule/tests/test_import.py b/common/lib/xmodule/tests/test_import.py
index 6643b93a67..dd55f8ff38 100644
--- a/common/lib/xmodule/tests/test_import.py
+++ b/common/lib/xmodule/tests/test_import.py
@@ -1,7 +1,8 @@
from path import path
-
import unittest
+from lxml import etree
+
from xmodule.x_module import XMLParsingSystem, XModuleDescriptor
from xmodule.errorhandlers import ignore_errors_handler
from xmodule.modulestore import Location
@@ -61,3 +62,26 @@ class ImportTestCase(unittest.TestCase):
self.assertEqual(descriptor.definition['data'],
re_import_descriptor.definition['data'])
+
+ def test_fixed_xml_tag(self):
+ """Make sure a tag that's been fixed exports as the original tag type"""
+
+ # create a malformed tag with valid xml contents
+ root = etree.Element('malformed')
+ good_xml = ''''''
+ root.text = good_xml
+
+ xml_str_in = etree.tostring(root)
+
+ # load it
+ system = self.get_system()
+ descriptor = XModuleDescriptor.load_from_xml(xml_str_in, system, 'org', 'course',
+ None)
+ # export it
+ resource_fs = None
+ xml_str_out = descriptor.export_to_xml(resource_fs)
+
+ # Now make sure the exported xml is a sequential
+ xml_out = etree.fromstring(xml_str_out)
+ self.assertEqual(xml_out.tag, 'sequential')
+
diff --git a/common/lib/xmodule/xmodule/malformed_module.py b/common/lib/xmodule/xmodule/malformed_module.py
index 54032cf7d2..ac419fff26 100644
--- a/common/lib/xmodule/xmodule/malformed_module.py
+++ b/common/lib/xmodule/xmodule/malformed_module.py
@@ -1,5 +1,6 @@
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
@@ -8,10 +9,18 @@ import logging
log = logging.getLogger(__name__)
+class MalformedModule(XModule):
+ def get_html(self):
+ '''Show an error.
+ TODO (vshnayder): proper style, divs, etc.
+ '''
+ return "Malformed content--not showing through get_html()"
+
class MalformedDescriptor(EditingDescriptor):
"""
Module that provides a raw editing view of broken xml.
"""
+ module_class = MalformedModule
@classmethod
def from_xml(cls, xml_data, system, org=None, course=None):
@@ -20,8 +29,8 @@ class MalformedDescriptor(EditingDescriptor):
Does not try to parse the data--just stores it.
'''
- #log.debug("processing '{0}'".format(xml_data))
try:
+ # If this is already a malformed tag, don't want to re-wrap it.
xml_obj = etree.fromstring(xml_data)
if xml_obj.tag == 'malformed':
xml_data = xml_obj.text
@@ -40,9 +49,18 @@ class MalformedDescriptor(EditingDescriptor):
def export_to_xml(self, resource_fs):
'''
- Export as a string wrapped in xml
- '''
- root = etree.Element('malformed')
- root.text = self.definition['data']
- return etree.tostring(root)
+ If the definition data is invalid xml, export it wrapped in a malformed
+ 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.
+ '''
+ try:
+ xml = etree.fromstring(self.definition['data'])
+ return etree.tostring(xml)
+ except etree.XMLSyntaxError:
+ # still not valid.
+ root = etree.Element('malformed')
+ root.text = self.definition['data']
+ return etree.tostring(root)
From 46775386d376e0cba2e2d146bce0e34e3984cfda Mon Sep 17 00:00:00 2001
From: Victor Shnayder
Date: Mon, 30 Jul 2012 18:29:18 -0400
Subject: [PATCH 037/124] make CACHE_TIMEOUT messages go away
---
cms/envs/dev.py | 3 +++
lms/envs/dev.py | 13 ++++++++-----
2 files changed, 11 insertions(+), 5 deletions(-)
diff --git a/cms/envs/dev.py b/cms/envs/dev.py
index 59bc623729..4098263829 100644
--- a/cms/envs/dev.py
+++ b/cms/envs/dev.py
@@ -76,3 +76,6 @@ CACHES = {
'KEY_FUNCTION': 'util.memcache.safe_key',
}
}
+
+# Make the keyedcache startup warnings go away
+CACHE_TIMEOUT = 0
diff --git a/lms/envs/dev.py b/lms/envs/dev.py
index 1a2659cb1f..3fd86f1aee 100644
--- a/lms/envs/dev.py
+++ b/lms/envs/dev.py
@@ -17,7 +17,7 @@ MITX_FEATURES['DISABLE_START_DATES'] = True
WIKI_ENABLED = True
-LOGGING = get_logger_config(ENV_ROOT / "log",
+LOGGING = get_logger_config(ENV_ROOT / "log",
logging_env="dev",
tracking_filename="tracking.log",
debug=True)
@@ -30,7 +30,7 @@ DATABASES = {
}
CACHES = {
- # This is the cache used for most things. Askbot will not work without a
+ # This is the cache used for most things. Askbot will not work without a
# functioning cache -- it relies on caching to load its settings in places.
# In staging/prod envs, the sessions also live here.
'default': {
@@ -52,11 +52,14 @@ CACHES = {
}
}
+# Make the keyedcache startup warnings go away
+CACHE_TIMEOUT = 0
+
# Dummy secret key for dev
SECRET_KEY = '85920908f28904ed733fe576320db18cabd7b6cd'
################################ DEBUG TOOLBAR #################################
-INSTALLED_APPS += ('debug_toolbar',)
+INSTALLED_APPS += ('debug_toolbar',)
MIDDLEWARE_CLASSES += ('debug_toolbar.middleware.DebugToolbarMiddleware',)
INTERNAL_IPS = ('127.0.0.1',)
@@ -71,8 +74,8 @@ DEBUG_TOOLBAR_PANELS = (
'debug_toolbar.panels.logger.LoggingPanel',
# Enabling the profiler has a weird bug as of django-debug-toolbar==0.9.4 and
-# Django=1.3.1/1.4 where requests to views get duplicated (your method gets
-# hit twice). So you can uncomment when you need to diagnose performance
+# Django=1.3.1/1.4 where requests to views get duplicated (your method gets
+# hit twice). So you can uncomment when you need to diagnose performance
# problems, but you shouldn't leave it on.
# 'debug_toolbar.panels.profiling.ProfilingDebugPanel',
)
From ed35cefa295ae4b8e798d86587cbf8c838289df6 Mon Sep 17 00:00:00 2001
From: Victor Shnayder
Date: Mon, 30 Jul 2012 18:36:02 -0400
Subject: [PATCH 038/124] Fix html file handling.
* html files are now stored as follows:
If the html file is valid xml, store as html/stuff.xml
If it's not, store as html/stuff.xml, which contains
,
and html/stuff.html, which actually contains the contents.
Warn if the contents are not parseable with lxml's html parser,
but don't error.
* for parseable html, strip out the html tag when storing, so that it isn't
rendered into the middle of a page
* lots of backcompat to deal with paths. Can go away soon.
* fix output ordering in clean_xml
---
common/lib/xmodule/html_checker.py | 14 +++
common/lib/xmodule/stringify.py | 20 ++++
common/lib/xmodule/tests/test_stringify.py | 9 ++
common/lib/xmodule/xmodule/html_module.py | 103 ++++++++++++++++--
common/lib/xmodule/xmodule/xml_module.py | 88 ++++++++-------
.../management/commands/clean_xml.py | 1 +
6 files changed, 184 insertions(+), 51 deletions(-)
create mode 100644 common/lib/xmodule/html_checker.py
create mode 100644 common/lib/xmodule/stringify.py
create mode 100644 common/lib/xmodule/tests/test_stringify.py
diff --git a/common/lib/xmodule/html_checker.py b/common/lib/xmodule/html_checker.py
new file mode 100644
index 0000000000..5e6b417d28
--- /dev/null
+++ b/common/lib/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/stringify.py b/common/lib/xmodule/stringify.py
new file mode 100644
index 0000000000..dad964140f
--- /dev/null
+++ b/common/lib/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/tests/test_stringify.py b/common/lib/xmodule/tests/test_stringify.py
new file mode 100644
index 0000000000..62d7683886
--- /dev/null
+++ b/common/lib/xmodule/tests/test_stringify.py
@@ -0,0 +1,9 @@
+from nose.tools import assert_equals
+from lxml import etree
+from stringify import stringify_children
+
+def test_stringify():
+ html = '''Hi
there Bruce!
'''
+ xml = etree.fromstring(html)
+ out = stringify_children(xml)
+ assert_equals(out, '''Hi
there Bruce!
''')
diff --git a/common/lib/xmodule/xmodule/html_module.py b/common/lib/xmodule/xmodule/html_module.py
index b9bc34aed6..22d3bf163f 100644
--- a/common/lib/xmodule/xmodule/html_module.py
+++ b/common/lib/xmodule/xmodule/html_module.py
@@ -1,13 +1,17 @@
+import copy
+from fs.errors import ResourceNotFoundError
import logging
import os
from lxml import etree
from xmodule.x_module import XModule
-from xmodule.raw_module import RawDescriptor
+from xmodule.xml_module import XmlDescriptor
+from xmodule.editing_module import EditingDescriptor
+from stringify import stringify_children
+from html_checker import check_html
log = logging.getLogger("mitx.courseware")
-
class HtmlModule(XModule):
def get_html(self):
return self.html
@@ -19,33 +23,108 @@ 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 definition_loader(cls, xml_object, system):
+ '''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):
+ log.warning("Couldn't parse html in {0}.".format(filepath))
+ return {'data' : html}
+ except (ResourceNotFoundError) as err:
+ msg = 'Unable to load file contents at path {0}: {1} '.format(
+ filepath, err)
+ log.error(msg)
+ raise
@classmethod
def split_to_file(cls, xml_object):
- # never include inline html
+ '''Never include inline html'''
return True
+
+
+ # TODO (vshnayder): make export put things in the right places.
+
+ def definition_to_xml(self, resource_fs):
+ '''If the contents are valid xml, write them to filename.xml. Otherwise,
+ write just the tag to filename.xml, and the html
+ string to filename.html.
+ '''
+ try:
+ return etree.fromstring(self.definition['data'])
+ except etree.XMLSyntaxError:
+ pass
+
+ # Not proper format. Write html to file, return an empty tag
+ filepath = u'{category}/{name}.html'.format(category=self.category,
+ name=self.name)
+
+ resource_fs.makedir(os.path.dirname(filepath), allow_recreate=True)
+ with resource_fs.open(filepath, 'w') as file:
+ file.write(self.definition['data'])
+
+ elt = etree.Element('html')
+ elt.set("filename", self.name)
+ return elt
+
diff --git a/common/lib/xmodule/xmodule/xml_module.py b/common/lib/xmodule/xmodule/xml_module.py
index 5786dbd227..188df21130 100644
--- a/common/lib/xmodule/xmodule/xml_module.py
+++ b/common/lib/xmodule/xmodule/xml_module.py
@@ -162,6 +162,52 @@ class XmlDescriptor(XModuleDescriptor):
"""
return etree.parse(file_object).getroot()
+ @classmethod
+ def definition_loader(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 (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)
+
+
@classmethod
def from_xml(cls, xml_data, system, org=None, course=None):
"""
@@ -191,44 +237,8 @@ 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.definition_loader(xml_object, system, location)
+ metadata = metadata_loader()
# VS[compat] -- just have the url_name lookup once translation is done
slug = xml_object.get('url_name', xml_object.get('slug'))
return cls(
@@ -283,7 +293,7 @@ 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
+ # Put this object in its own file
filepath = self.__class__._format_filepath(self.category, self.name)
resource_fs.makedir(os.path.dirname(filepath), allow_recreate=True)
with resource_fs.open(filepath, 'w') as file:
diff --git a/lms/djangoapps/courseware/management/commands/clean_xml.py b/lms/djangoapps/courseware/management/commands/clean_xml.py
index 7523fd8373..41f0c39cde 100644
--- a/lms/djangoapps/courseware/management/commands/clean_xml.py
+++ b/lms/djangoapps/courseware/management/commands/clean_xml.py
@@ -143,6 +143,7 @@ def check_roundtrip(course_dir):
# dircmp doesn't do recursive diffs.
# diff = dircmp(course_dir, export_dir, ignore=[], hide=[])
print "======== Roundtrip diff: ========="
+ sys.stdout.flush() # needed to make diff appear in the right place
os.system("diff -r {0} {1}".format(course_dir, export_dir))
print "======== ideally there is no diff above this ======="
From 0ae434cc092617b970fee3968084c378d609c4f2 Mon Sep 17 00:00:00 2001
From: Victor Shnayder
Date: Tue, 31 Jul 2012 11:24:39 -0400
Subject: [PATCH 039/124] Move path_to_location out of mongo.py
* also bugfix for load_definition in html_module
* a bit of refactoring of Location checking code in mongo.py
---
common/lib/xmodule/xmodule/html_module.py | 2 +-
.../xmodule/xmodule/modulestore/__init__.py | 47 +++----
.../lib/xmodule/xmodule/modulestore/mongo.py | 120 +++---------------
.../lib/xmodule/xmodule/modulestore/search.py | 96 ++++++++++++++
.../xmodule/modulestore/tests/test_mongo.py | 15 +--
common/lib/xmodule/xmodule/xml_module.py | 8 +-
lms/djangoapps/courseware/views.py | 3 +-
7 files changed, 155 insertions(+), 136 deletions(-)
create mode 100644 common/lib/xmodule/xmodule/modulestore/search.py
diff --git a/common/lib/xmodule/xmodule/html_module.py b/common/lib/xmodule/xmodule/html_module.py
index 22d3bf163f..996d3c4ead 100644
--- a/common/lib/xmodule/xmodule/html_module.py
+++ b/common/lib/xmodule/xmodule/html_module.py
@@ -54,7 +54,7 @@ class HtmlDescriptor(XmlDescriptor, EditingDescriptor):
# snippets that will be included in the middle of pages.
@classmethod
- def definition_loader(cls, xml_object, system):
+ 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
diff --git a/common/lib/xmodule/xmodule/modulestore/__init__.py b/common/lib/xmodule/xmodule/modulestore/__init__.py
index 279782b61a..a0167d781b 100644
--- a/common/lib/xmodule/xmodule/modulestore/__init__.py
+++ b/common/lib/xmodule/xmodule/modulestore/__init__.py
@@ -5,7 +5,7 @@ that are stored in a database an accessible using their Location as an identifie
import re
from collections import namedtuple
-from .exceptions import InvalidLocationError
+from .exceptions import InvalidLocationError, InsufficientSpecificationError
import logging
log = logging.getLogger('mitx.' + 'modulestore')
@@ -38,15 +38,15 @@ class Location(_LocationBase):
'''
__slots__ = ()
- @classmethod
- def clean(cls, value):
+ @staticmethod
+ def clean(value):
"""
Return value, made into a form legal for locations
"""
return re.sub('_+', '_', INVALID_CHARS.sub('_', value))
- @classmethod
- def is_valid(cls, value):
+ @staticmethod
+ def is_valid(value):
'''
Check if the value is a valid location, in any acceptable format.
'''
@@ -56,6 +56,21 @@ class Location(_LocationBase):
return False
return True
+ @staticmethod
+ def ensure_fully_specified(location):
+ '''Make sure location is valid, and fully specified. Raises
+ InvalidLocationError or InsufficientSpecificationError if not.
+
+ returns a Location object corresponding to location.
+ '''
+ loc = Location(location)
+ for key, val in loc.dict().iteritems():
+ if key != 'revision' and val is None:
+ raise InsufficientSpecificationError(location)
+ return loc
+
+
+
def __new__(_cls, loc_or_tag=None, org=None, course=None, category=None,
name=None, revision=None):
"""
@@ -254,25 +269,11 @@ 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.
+ def get_parent_locations(self, location):
+ '''Find all locations that are the parents of this location. Needed
+ for path_to_location().
- raise ItemNotFoundError if the location doesn't exist.
-
- If course, chapter, section are not None, restrict search to paths with those
- components as specified.
-
- raise NoPathToItem if the location exists, but isn't accessible via
- a path that matches the course/chapter/section restrictions.
-
- In general, a location may be accessible via many paths. This method may
- return any valid path.
-
- Return a tuple (course, chapter, section, position).
-
- If the section a sequence, position should be the position of this location
- in that sequence. Otherwise, position should be None.
+ returns an iterable of things that can be passed to Location.
'''
raise NotImplementedError
diff --git a/common/lib/xmodule/xmodule/modulestore/mongo.py b/common/lib/xmodule/xmodule/modulestore/mongo.py
index df4e20f3a7..061e2aafe9 100644
--- a/common/lib/xmodule/xmodule/modulestore/mongo.py
+++ b/common/lib/xmodule/xmodule/modulestore/mongo.py
@@ -9,11 +9,10 @@ from importlib import import_module
from xmodule.errorhandlers import strict_error_handler
from xmodule.x_module import XModuleDescriptor
from xmodule.mako_module import MakoDescriptorSystem
-from xmodule.course_module import CourseDescriptor
from mitxmako.shortcuts import render_to_string
from . import ModuleStore, Location
-from .exceptions import (ItemNotFoundError, InsufficientSpecificationError,
+from .exceptions import (ItemNotFoundError,
NoPathToItem, DuplicateItemError)
# TODO (cpennington): This code currently operates under the assumption that
@@ -172,12 +171,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 +201,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 +280,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)
+ 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': str(location)},
{'_id': True})
return [i['_id'] for i in items]
- def path_to_location(self, location, course_name=None):
- '''
- Try to find a course_id/chapter/section[/position] path to this location.
- The courseware insists that the first level in the course is chapter,
- but any kind of module can be a "section".
-
- location: something that can be passed to Location
- course_name: [optional]. If not None, restrict search to paths
- in that course.
-
- raise ItemNotFoundError if the location doesn't exist.
-
- raise NoPathToItem if the location exists, but isn't accessible via
- a chapter/section path in the course(s) being searched.
-
- Return a tuple (course_id, chapter, section, position) suitable for the
- courseware index view.
-
- A location may be accessible via many paths. This method may
- return any valid path.
-
- If the section is a sequence, position will be the position
- of this location in that sequence. Otherwise, position will
- be None. TODO (vshnayder): Not true yet.
- '''
- # Check that location is present at all
- if self._find_one(location) is None:
- raise ItemNotFoundError(location)
-
- def flatten(xs):
- '''Convert lisp-style (a, (b, (c, ()))) lists into a python list.
- Not a general flatten function. '''
- p = []
- while xs != ():
- p.append(xs[0])
- xs = xs[1]
- return p
-
- def find_path_to_course(location, course_name=None):
- '''Find a path up the location graph to a node with the
- specified category. If no path exists, return None. If a
- path exists, return it as a list with target location
- first, and the starting location last.
- '''
- # Standard DFS
-
- # To keep track of where we came from, the work queue has
- # tuples (location, path-so-far). To avoid lots of
- # copying, the path-so-far is stored as a lisp-style
- # list--nested hd::tl tuples, and flattened at the end.
- queue = [(location, ())]
- while len(queue) > 0:
- (loc, path) = queue.pop() # Takes from the end
- loc = Location(loc)
- # print 'Processing loc={0}, path={1}'.format(loc, path)
- if loc.category == "course":
- if course_name is None or course_name == loc.name:
- # Found it!
- path = (loc, path)
- return flatten(path)
-
- # otherwise, add parent locations at the end
- newpath = (loc, path)
- parents = self.get_parent_locations(loc)
- queue.extend(zip(parents, repeat(newpath)))
-
- # If we're here, there is no path
- return None
-
- path = find_path_to_course(location, course_name)
- if path is None:
- raise(NoPathToItem(location))
-
- n = len(path)
- course_id = CourseDescriptor.location_to_id(path[0])
- chapter = path[1].name if n > 1 else None
- section = path[2].name if n > 2 else None
-
- # TODO (vshnayder): not handling position at all yet...
- position = None
-
- return (course_id, chapter, section, position)
diff --git a/common/lib/xmodule/xmodule/modulestore/search.py b/common/lib/xmodule/xmodule/modulestore/search.py
new file mode 100644
index 0000000000..a383b3f8ec
--- /dev/null
+++ b/common/lib/xmodule/xmodule/modulestore/search.py
@@ -0,0 +1,96 @@
+from itertools import repeat
+
+from xmodule.course_module import CourseDescriptor
+
+from .exceptions import (ItemNotFoundError, NoPathToItem)
+from . import ModuleStore, Location
+
+
+def path_to_location(modulestore, location, course_name=None):
+ '''
+ Try to find a course_id/chapter/section[/position] path to location in
+ modulestore. The courseware insists that the first level in the course is
+ chapter, but any kind of module can be a "section".
+
+ location: something that can be passed to Location
+ course_name: [optional]. If not None, restrict search to paths
+ in that course.
+
+ raise ItemNotFoundError if the location doesn't exist.
+
+ raise NoPathToItem if the location exists, but isn't accessible via
+ a chapter/section path in the course(s) being searched.
+
+ Return a tuple (course_id, chapter, section, position) suitable for the
+ courseware index view.
+
+ A location may be accessible via many paths. This method may
+ return any valid path.
+
+ If the section is a sequence, position will be the position
+ of this location in that sequence. Otherwise, position will
+ be None. TODO (vshnayder): Not true yet.
+ '''
+
+ def flatten(xs):
+ '''Convert lisp-style (a, (b, (c, ()))) list into a python list.
+ Not a general flatten function. '''
+ p = []
+ while xs != ():
+ p.append(xs[0])
+ xs = xs[1]
+ return p
+
+ def find_path_to_course(location, course_name=None):
+ '''Find a path up the location graph to a node with the
+ specified category.
+
+ If no path exists, return None.
+
+ If a path exists, return it as a list with target location first, and
+ the starting location last.
+ '''
+ # Standard DFS
+
+ # To keep track of where we came from, the work queue has
+ # tuples (location, path-so-far). To avoid lots of
+ # copying, the path-so-far is stored as a lisp-style
+ # list--nested hd::tl tuples, and flattened at the end.
+ queue = [(location, ())]
+ while len(queue) > 0:
+ (loc, path) = queue.pop() # Takes from the end
+ loc = Location(loc)
+
+ # get_parent_locations should raise ItemNotFoundError if location
+ # isn't found so we don't have to do it explicitly. Call this
+ # first to make sure the location is there (even if it's a course, and
+ # we would otherwise immediately exit).
+ parents = modulestore.get_parent_locations(loc)
+
+ # print 'Processing loc={0}, path={1}'.format(loc, path)
+ if loc.category == "course":
+ if course_name is None or course_name == loc.name:
+ # Found it!
+ path = (loc, path)
+ return flatten(path)
+
+ # otherwise, add parent locations at the end
+ newpath = (loc, path)
+ queue.extend(zip(parents, repeat(newpath)))
+
+ # If we're here, there is no path
+ return None
+
+ path = find_path_to_course(location, course_name)
+ if path is None:
+ raise(NoPathToItem(location))
+
+ n = len(path)
+ course_id = CourseDescriptor.location_to_id(path[0])
+ chapter = path[1].name if n > 1 else None
+ section = path[2].name if n > 2 else None
+
+ # TODO (vshnayder): not handling position at all yet...
+ position = None
+
+ return (course_id, chapter, section, position)
diff --git a/common/lib/xmodule/xmodule/modulestore/tests/test_mongo.py b/common/lib/xmodule/xmodule/modulestore/tests/test_mongo.py
index cb2bc6e20c..24f0441ee0 100644
--- a/common/lib/xmodule/xmodule/modulestore/tests/test_mongo.py
+++ b/common/lib/xmodule/xmodule/modulestore/tests/test_mongo.py
@@ -8,6 +8,7 @@ from xmodule.modulestore import Location
from xmodule.modulestore.exceptions import InvalidLocationError, ItemNotFoundError, NoPathToItem
from xmodule.modulestore.mongo import MongoModuleStore
from xmodule.modulestore.xml_importer import import_from_xml
+from xmodule.modulestore.search import path_to_location
# from ~/mitx_all/mitx/common/lib/xmodule/xmodule/modulestore/tests/
# to ~/mitx_all/mitx/common/test
@@ -28,7 +29,7 @@ DEFAULT_CLASS = 'xmodule.raw_module.RawDescriptor'
class TestMongoModuleStore(object):
-
+ '''Tests!'''
@classmethod
def setupClass(cls):
cls.connection = pymongo.connection.Connection(HOST, PORT)
@@ -67,7 +68,7 @@ class TestMongoModuleStore(object):
def test_init(self):
'''Make sure the db loads, and print all the locations in the db.
- Call this directly from failing tests to see what's loaded'''
+ Call this directly from failing tests to see what is loaded'''
ids = list(self.connection[DB][COLLECTION].find({}, {'_id': True}))
pprint([Location(i['_id']).url() for i in ids])
@@ -93,8 +94,6 @@ class TestMongoModuleStore(object):
self.store.get_item("i4x://edX/toy/video/Welcome"),
None)
-
-
def test_find_one(self):
assert_not_equals(
self.store._find_one(Location("i4x://edX/toy/course/2012_Fall")),
@@ -117,13 +116,13 @@ class TestMongoModuleStore(object):
("edX/toy/2012_Fall", "Overview", "Toy_Videos", None)),
)
for location, expected in should_work:
- assert_equals(self.store.path_to_location(location), expected)
+ assert_equals(path_to_location(self.store, location), expected)
not_found = (
- "i4x://edX/toy/video/WelcomeX",
+ "i4x://edX/toy/video/WelcomeX", "i4x://edX/toy/course/NotHome"
)
for location in not_found:
- assert_raises(ItemNotFoundError, self.store.path_to_location, location)
+ assert_raises(ItemNotFoundError, path_to_location, self.store, location)
# Since our test files are valid, there shouldn't be any
# elements with no path to them. But we can look for them in
@@ -132,5 +131,5 @@ class TestMongoModuleStore(object):
"i4x://edX/simple/video/Lost_Video",
)
for location in no_path:
- assert_raises(NoPathToItem, self.store.path_to_location, location, "toy")
+ assert_raises(NoPathToItem, path_to_location, self.store, location, "toy")
diff --git a/common/lib/xmodule/xmodule/xml_module.py b/common/lib/xmodule/xmodule/xml_module.py
index 188df21130..de62bc5d38 100644
--- a/common/lib/xmodule/xmodule/xml_module.py
+++ b/common/lib/xmodule/xmodule/xml_module.py
@@ -163,7 +163,7 @@ class XmlDescriptor(XModuleDescriptor):
return etree.parse(file_object).getroot()
@classmethod
- def definition_loader(cls, xml_object, system, location):
+ 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)'''
@@ -225,7 +225,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)
@@ -237,8 +237,8 @@ class XmlDescriptor(XModuleDescriptor):
metadata[attr_map.metadata_key] = attr_map.to_metadata(val)
return metadata
- definition = cls.definition_loader(xml_object, system, location)
- metadata = metadata_loader()
+ 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(
diff --git a/lms/djangoapps/courseware/views.py b/lms/djangoapps/courseware/views.py
index 7281ab01ad..a52f715efd 100644
--- a/lms/djangoapps/courseware/views.py
+++ b/lms/djangoapps/courseware/views.py
@@ -20,6 +20,7 @@ from module_render import toc_for_course, get_module, get_section
from models import StudentModuleCache
from student.models import UserProfile
from xmodule.modulestore import Location
+from xmodule.modulestore.search import path_to_location
from xmodule.modulestore.exceptions import InvalidLocationError, ItemNotFoundError, NoPathToItem
from xmodule.modulestore.django import modulestore
from xmodule.course_module import CourseDescriptor
@@ -233,7 +234,7 @@ def jump_to(request, location):
# Complain if there's not data for this location
try:
- (course_id, chapter, section, position) = modulestore().path_to_location(location)
+ (course_id, chapter, section, position) = path_to_location(modulestore(), location)
except ItemNotFoundError:
raise Http404("No data at this location: {0}".format(location))
except NoPathToItem:
From 0edc40de34f37c3322e440656e71d7dbe77c202a Mon Sep 17 00:00:00 2001
From: Victor Shnayder
Date: Tue, 31 Jul 2012 11:25:10 -0400
Subject: [PATCH 040/124] Address minor comments in #313
---
common/lib/xmodule/xmodule/modulestore/mongo.py | 2 +-
common/lib/xmodule/xmodule/x_module.py | 3 ---
.../courseware/management/commands/clean_xml.py | 3 ---
lms/djangoapps/courseware/tests/tests.py | 14 ++++++++------
4 files changed, 9 insertions(+), 13 deletions(-)
diff --git a/common/lib/xmodule/xmodule/modulestore/mongo.py b/common/lib/xmodule/xmodule/modulestore/mongo.py
index 061e2aafe9..6a0bdad06a 100644
--- a/common/lib/xmodule/xmodule/modulestore/mongo.py
+++ b/common/lib/xmodule/xmodule/modulestore/mongo.py
@@ -293,7 +293,7 @@ class MongoModuleStore(ModuleStore):
# Check that it's actually in this modulestore.
item = self._find_one(location)
# now get the parents
- items = self.collection.find({'definition.children': str(location)},
+ items = self.collection.find({'definition.children': location.url()},
{'_id': True})
return [i['_id'] for i in items]
diff --git a/common/lib/xmodule/xmodule/x_module.py b/common/lib/xmodule/xmodule/x_module.py
index 8803c37d78..c4ba3e1e58 100644
--- a/common/lib/xmodule/xmodule/x_module.py
+++ b/common/lib/xmodule/xmodule/x_module.py
@@ -194,9 +194,6 @@ class XModule(HTMLSnippet):
self.metadata = kwargs.get('metadata', {})
self._loaded_children = None
- def get_name(self):
- return self.name
-
def get_children(self):
'''
Return module instances for all the children of this module.
diff --git a/lms/djangoapps/courseware/management/commands/clean_xml.py b/lms/djangoapps/courseware/management/commands/clean_xml.py
index 41f0c39cde..05865bb289 100644
--- a/lms/djangoapps/courseware/management/commands/clean_xml.py
+++ b/lms/djangoapps/courseware/management/commands/clean_xml.py
@@ -17,9 +17,6 @@ def traverse_tree(course):
queue = [course]
while len(queue) > 0:
node = queue.pop()
-# print '{0}:'.format(node.location)
-# if 'data' in node.definition:
-# print '{0}'.format(node.definition['data'])
queue.extend(node.get_children())
return True
diff --git a/lms/djangoapps/courseware/tests/tests.py b/lms/djangoapps/courseware/tests/tests.py
index 8e9d13f8d5..0e2f7d7aa5 100644
--- a/lms/djangoapps/courseware/tests/tests.py
+++ b/lms/djangoapps/courseware/tests/tests.py
@@ -1,19 +1,20 @@
import copy
import json
+from path import path
import os
from pprint import pprint
+from nose import SkipTest
from django.test import TestCase
from django.test.client import Client
-from mock import patch, Mock
-from override_settings import override_settings
from django.conf import settings
from django.core.urlresolvers import reverse
-from path import path
+from mock import patch, Mock
+from override_settings import override_settings
-from student.models import Registration
from django.contrib.auth.models import User
+from student.models import Registration
from xmodule.modulestore.django import modulestore
import xmodule.modulestore.django
@@ -189,11 +190,12 @@ class RealCoursesLoadTestCase(PageLoader):
xmodule.modulestore.django._MODULESTORES = {}
xmodule.modulestore.django.modulestore().collection.drop()
- # TODO: Disabled test for now.. Fix once things are cleaned up.
- def Xtest_real_courses_loads(self):
+ def test_real_courses_loads(self):
'''See if any real courses are available at the REAL_DATA_DIR.
If they are, check them.'''
+ # TODO: Disabled test for now.. Fix once things are cleaned up.
+ raise SkipTest
# TODO: adjust staticfiles_dirs
if not os.path.isdir(REAL_DATA_DIR):
# No data present. Just pass.
From 707551b08d25bcf92572a323176aeeec0398aead Mon Sep 17 00:00:00 2001
From: Victor Shnayder
Date: Tue, 31 Jul 2012 11:26:02 -0400
Subject: [PATCH 041/124] local variable naming tweak
---
common/lib/xmodule/xmodule/modulestore/xml.py | 13 +++++++------
1 file changed, 7 insertions(+), 6 deletions(-)
diff --git a/common/lib/xmodule/xmodule/modulestore/xml.py b/common/lib/xmodule/xmodule/modulestore/xml.py
index 8a084b19ee..b54963b513 100644
--- a/common/lib/xmodule/xmodule/modulestore/xml.py
+++ b/common/lib/xmodule/xmodule/modulestore/xml.py
@@ -70,18 +70,19 @@ 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
From c0cdff7071431fb522a05b596bd0aa07232d1584 Mon Sep 17 00:00:00 2001
From: Victor Shnayder
Date: Tue, 31 Jul 2012 13:44:04 -0400
Subject: [PATCH 042/124] Rename MalformedDescriptor to ErrorDescriptor
* change references and tests
* add staff/non-staff display
* added is_staff to ModuleSystem
---
cms/djangoapps/contentstore/views.py | 5 ++-
common/lib/xmodule/setup.py | 2 +-
common/lib/xmodule/tests/__init__.py | 3 +-
common/lib/xmodule/tests/test_import.py | 12 +++---
.../{malformed_module.py => error_module.py} | 41 +++++++++++--------
common/lib/xmodule/xmodule/x_module.py | 26 ++++++++----
lms/djangoapps/courseware/module_render.py | 1 +
7 files changed, 58 insertions(+), 32 deletions(-)
rename common/lib/xmodule/xmodule/{malformed_module.py => error_module.py} (58%)
diff --git a/cms/djangoapps/contentstore/views.py b/cms/djangoapps/contentstore/views.py
index 6d5b2117c4..111c91e03a 100644
--- a/cms/djangoapps/contentstore/views.py
+++ b/cms/djangoapps/contentstore/views.py
@@ -214,7 +214,10 @@ def preview_module_system(request, preview_id, descriptor):
get_module=partial(get_preview_module, request, preview_id),
render_template=render_from_lms,
debug=True,
- replace_urls=replace_urls
+ replace_urls=replace_urls,
+ # TODO (vshnayder): CMS users won't see staff view unless they are CMS staff.
+ # is that what we want?
+ is_staff=request.user.is_staff
)
diff --git a/common/lib/xmodule/setup.py b/common/lib/xmodule/setup.py
index 09a6b7e712..8a0a6bb139 100644
--- a/common/lib/xmodule/setup.py
+++ b/common/lib/xmodule/setup.py
@@ -25,7 +25,7 @@ setup(
"discuss = xmodule.backcompat_module:TranslateCustomTagDescriptor",
"html = xmodule.html_module:HtmlDescriptor",
"image = xmodule.backcompat_module:TranslateCustomTagDescriptor",
- "malformed = xmodule.malformed_module:MalformedDescriptor",
+ "error = xmodule.error_module:ErrorDescriptor",
"problem = xmodule.capa_module:CapaDescriptor",
"problemset = xmodule.vertical_module:VerticalDescriptor",
"section = xmodule.backcompat_module:SemanticSectionDescriptor",
diff --git a/common/lib/xmodule/tests/__init__.py b/common/lib/xmodule/tests/__init__.py
index 94bf1da65e..7f6fcfe00c 100644
--- a/common/lib/xmodule/tests/__init__.py
+++ b/common/lib/xmodule/tests/__init__.py
@@ -31,7 +31,8 @@ i4xs = ModuleSystem(
user=Mock(),
filestore=fs.osfs.OSFS(os.path.dirname(os.path.realpath(__file__))),
debug=True,
- xqueue_callback_url='/'
+ xqueue_callback_url='/',
+ is_staff=False
)
diff --git a/common/lib/xmodule/tests/test_import.py b/common/lib/xmodule/tests/test_import.py
index dd55f8ff38..497354a18e 100644
--- a/common/lib/xmodule/tests/test_import.py
+++ b/common/lib/xmodule/tests/test_import.py
@@ -33,7 +33,7 @@ class ImportTestCase(unittest.TestCase):
return system
def test_fallback(self):
- '''Make sure that malformed xml loads as a MalformedDescriptorb.'''
+ '''Make sure that malformed xml loads as an ErrorDescriptor.'''
bad_xml = ''''''
@@ -43,10 +43,10 @@ class ImportTestCase(unittest.TestCase):
None)
self.assertEqual(descriptor.__class__.__name__,
- 'MalformedDescriptor')
+ 'ErrorDescriptor')
def test_reimport(self):
- '''Make sure an already-exported malformed xml tag loads properly'''
+ '''Make sure an already-exported error xml tag loads properly'''
bad_xml = ''''''
system = self.get_system()
@@ -58,7 +58,7 @@ class ImportTestCase(unittest.TestCase):
'org', 'course',
None)
self.assertEqual(re_import_descriptor.__class__.__name__,
- 'MalformedDescriptor')
+ 'ErrorDescriptor')
self.assertEqual(descriptor.definition['data'],
re_import_descriptor.definition['data'])
@@ -66,8 +66,8 @@ class ImportTestCase(unittest.TestCase):
def test_fixed_xml_tag(self):
"""Make sure a tag that's been fixed exports as the original tag type"""
- # create a malformed tag with valid xml contents
- root = etree.Element('malformed')
+ # create a error tag with valid xml contents
+ root = etree.Element('error')
good_xml = ''''''
root.text = good_xml
diff --git a/common/lib/xmodule/xmodule/malformed_module.py b/common/lib/xmodule/xmodule/error_module.py
similarity index 58%
rename from common/lib/xmodule/xmodule/malformed_module.py
rename to common/lib/xmodule/xmodule/error_module.py
index ac419fff26..ee05e9c879 100644
--- a/common/lib/xmodule/xmodule/malformed_module.py
+++ b/common/lib/xmodule/xmodule/error_module.py
@@ -9,18 +9,26 @@ import logging
log = logging.getLogger(__name__)
-class MalformedModule(XModule):
+class ErrorModule(XModule):
def get_html(self):
'''Show an error.
TODO (vshnayder): proper style, divs, etc.
'''
- return "Malformed content--not showing through get_html()"
+ if not self.system.is_staff:
+ return self.system.render_template('module-error.html')
-class MalformedDescriptor(EditingDescriptor):
+ # staff get to see all the details
+ return self.system.render_template('module-error-staff.html', {
+ 'data' : self.definition['data'],
+ # TODO (vshnayder): need to get non-syntax errors in here somehow
+ 'error' : self.definition.get('error', 'Error not available')
+ })
+
+class ErrorDescriptor(EditingDescriptor):
"""
Module that provides a raw editing view of broken xml.
"""
- module_class = MalformedModule
+ module_class = ErrorModule
@classmethod
def from_xml(cls, xml_data, system, org=None, course=None):
@@ -29,38 +37,39 @@ class MalformedDescriptor(EditingDescriptor):
Does not try to parse the data--just stores it.
'''
+ definition = {}
try:
- # If this is already a malformed tag, don't want to re-wrap it.
+ # If this is already an error tag, don't want to re-wrap it.
xml_obj = etree.fromstring(xml_data)
- if xml_obj.tag == 'malformed':
+ if xml_obj.tag == 'error':
xml_data = xml_obj.text
- # TODO (vshnayder): how does one get back from this to a valid descriptor?
- # For now, have to fix manually.
- except etree.XMLSyntaxError:
- pass
+ except etree.XMLSyntaxError as err:
+ # Save the error to display later
+ definition['error'] = str(err)
- definition = { 'data' : xml_data }
- # TODO (vshnayder): Do we need a valid slug here? Just pick a random
+
+ definition['data'] = xml_data
+ # TODO (vshnayder): Do we need a unique slug here? Just pick a random
# 64-bit num?
- location = ['i4x', org, course, 'malformed', 'slug']
+ 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 a malformed
+ 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.
+ files, etc. That would just get re-wrapped on import.
'''
try:
xml = etree.fromstring(self.definition['data'])
return etree.tostring(xml)
except etree.XMLSyntaxError:
# still not valid.
- root = etree.Element('malformed')
+ root = etree.Element('error')
root.text = self.definition['data']
return etree.tostring(root)
diff --git a/common/lib/xmodule/xmodule/x_module.py b/common/lib/xmodule/xmodule/x_module.py
index c4ba3e1e58..2d7447b5f6 100644
--- a/common/lib/xmodule/xmodule/x_module.py
+++ b/common/lib/xmodule/xmodule/x_module.py
@@ -455,14 +455,14 @@ class XModuleDescriptor(Plugin, HTMLSnippet):
descriptor = class_.from_xml(xml_data, system, org, course)
except (ResourceNotFoundError, XMLSyntaxError) as err:
- # Didn't load properly. Fall back on loading as a malformed
+ # 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.malformed_module import MalformedDescriptor
+ from xmodule.error_module import ErrorDescriptor
#system.error_handler("Error loading from xml.")
- descriptor = MalformedDescriptor.from_xml(xml_data, system, org, course)
+ descriptor = ErrorDescriptor.from_xml(xml_data, system, org, course)
return descriptor
@@ -597,10 +597,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, xqueue_default_queuename="null"):
+ 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.
@@ -626,6 +634,9 @@ 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
@@ -637,6 +648,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/lms/djangoapps/courseware/module_render.py b/lms/djangoapps/courseware/module_render.py
index 80a4ef90fc..1ff39f602e 100644
--- a/lms/djangoapps/courseware/module_render.py
+++ b/lms/djangoapps/courseware/module_render.py
@@ -172,6 +172,7 @@ def get_module(user, request, location, student_module_cache, position=None):
# a module is coming through get_html and is therefore covered
# by the replace_static_urls code below
replace_urls=replace_urls,
+ is_staff=user.is_staff,
)
# pass position specified in URL to module through ModuleSystem
system.set('position', position)
From e2e524453faa235ac90fac008839d6175dbb7379 Mon Sep 17 00:00:00 2001
From: Victor Shnayder
Date: Tue, 31 Jul 2012 13:44:40 -0400
Subject: [PATCH 043/124] Make courseware index view more bulletproof
---
lms/djangoapps/courseware/views.py | 68 +++++++++++++++++++-----------
1 file changed, 44 insertions(+), 24 deletions(-)
diff --git a/lms/djangoapps/courseware/views.py b/lms/djangoapps/courseware/views.py
index a52f715efd..fb4d2942f9 100644
--- a/lms/djangoapps/courseware/views.py
+++ b/lms/djangoapps/courseware/views.py
@@ -184,34 +184,54 @@ def index(request, course_id, chapter=None, section=None,
chapter = clean(chapter)
section = clean(section)
- context = {
- 'csrf': csrf(request)['csrf_token'],
- 'accordion': render_accordion(request, course, chapter, section),
- 'COURSE_TITLE': course.title,
- 'course': course,
- 'init': '',
- 'content': ''
- }
+ try:
+ context = {
+ 'csrf': csrf(request)['csrf_token'],
+ 'accordion': render_accordion(request, course, chapter, section),
+ 'COURSE_TITLE': course.title,
+ 'course': course,
+ 'init': '',
+ 'content': ''
+ }
- look_for_module = chapter is not None and section is not None
- if look_for_module:
- # TODO (cpennington): Pass the right course in here
+ look_for_module = chapter is not None and section is not None
+ if look_for_module:
+ # TODO (cpennington): Pass the right course in here
- section_descriptor = get_section(course, chapter, section)
- if section_descriptor is not None:
- student_module_cache = StudentModuleCache(request.user,
- section_descriptor)
- module, _, _, _ = get_module(request.user, request,
- section_descriptor.location,
- student_module_cache)
- context['content'] = module.get_html()
+ section_descriptor = get_section(course, chapter, section)
+ if section_descriptor is not None:
+ student_module_cache = StudentModuleCache(request.user,
+ section_descriptor)
+ module, _, _, _ = get_module(request.user, request,
+ section_descriptor.location,
+ student_module_cache)
+ context['content'] = module.get_html()
+ else:
+ log.warning("Couldn't find a section descriptor for course_id '{0}',"
+ "chapter '{1}', section '{2}'".format(
+ course_id, chapter, section))
+
+
+ result = render_to_response('courseware.html', context)
+ except:
+ # In production, don't want to let a 500 out for any reason
+ if settings.DEBUG:
+ raise
else:
- log.warning("Couldn't find a section descriptor for course_id '{0}',"
- "chapter '{1}', section '{2}'".format(
- course_id, chapter, section))
+ log.exception("Error in index view: user={user}, course={course},"
+ " chapter={chapter} section={section}"
+ "position={position}".format(
+ user=request.user,
+ course=course,
+ chapter=chapter,
+ section=section,
+ position=position
+ ))
+ try:
+ result = render_to_response('courseware-error.html', {})
+ except:
+ result = HttpResponse("There was an unrecoverable error")
-
- result = render_to_response('courseware.html', context)
return result
@ensure_csrf_cookie
From bc14441a75c010dd98c9389ad9d83015f91cc091 Mon Sep 17 00:00:00 2001
From: Victor Shnayder
Date: Tue, 31 Jul 2012 13:45:13 -0400
Subject: [PATCH 044/124] add template I forgot
---
lms/templates/module-error-staff.html | 11 +++++++++++
1 file changed, 11 insertions(+)
create mode 100644 lms/templates/module-error-staff.html
diff --git a/lms/templates/module-error-staff.html b/lms/templates/module-error-staff.html
new file mode 100644
index 0000000000..955f413db3
--- /dev/null
+++ b/lms/templates/module-error-staff.html
@@ -0,0 +1,11 @@
+
+
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.
+
+
Staff-only details below:
+
+
Error: ${error}
+
+
Raw data: ${data}
+
+
From 119ab639d0c816889a59c36540212eb1daed33fd Mon Sep 17 00:00:00 2001
From: Victor Shnayder
Date: Tue, 31 Jul 2012 17:12:00 -0400
Subject: [PATCH 045/124] make cms users look like staff by default
* so they get the staff view of problems
* does NOT actually set User.is_staff
---
cms/djangoapps/contentstore/views.py | 4 ++--
1 file changed, 2 insertions(+), 2 deletions(-)
diff --git a/cms/djangoapps/contentstore/views.py b/cms/djangoapps/contentstore/views.py
index 111c91e03a..0fccc2498b 100644
--- a/cms/djangoapps/contentstore/views.py
+++ b/cms/djangoapps/contentstore/views.py
@@ -215,9 +215,9 @@ def preview_module_system(request, preview_id, descriptor):
render_template=render_from_lms,
debug=True,
replace_urls=replace_urls,
- # TODO (vshnayder): CMS users won't see staff view unless they are CMS staff.
+ # TODO (vshnayder): All CMS users get staff view by default
# is that what we want?
- is_staff=request.user.is_staff
+ is_staff=True,
)
From 58543bd84b451c1da7497633e9550a3d6538e0c1 Mon Sep 17 00:00:00 2001
From: Victor Shnayder
Date: Tue, 31 Jul 2012 17:12:47 -0400
Subject: [PATCH 046/124] definition and metadata no longer lazy-loaded
* get rid of dump to json
* formatting
---
common/djangoapps/xmodule_modifiers.py | 11 +++++------
1 file changed, 5 insertions(+), 6 deletions(-)
diff --git a/common/djangoapps/xmodule_modifiers.py b/common/djangoapps/xmodule_modifiers.py
index d0d1e0be15..b942d6726b 100644
--- a/common/djangoapps/xmodule_modifiers.py
+++ b/common/djangoapps/xmodule_modifiers.py
@@ -76,8 +76,9 @@ def add_histogram(get_html, module):
histogram = grade_histogram(module_id)
render_histogram = len(histogram) > 0
- # TODO: fixme - no filename in module.xml in general (this code block for edx4edx)
- # the following if block is for summer 2012 edX course development; it will change when the CMS comes online
+ # TODO: fixme - no filename in module.xml in general (this code block
+ # for edx4edx) the following if block is for summer 2012 edX course
+ # development; it will change when the CMS comes online
if settings.MITX_FEATURES.get('DISPLAY_EDIT_LINK') and settings.DEBUG and module_xml.get('filename') is not None:
coursename = multicourse_settings.get_coursename_from_request(request)
github_url = multicourse_settings.get_course_github_url(coursename)
@@ -88,10 +89,8 @@ def add_histogram(get_html, module):
else:
edit_link = False
- # Cast module.definition and module.metadata to dicts so that json can dump them
- # even though they are lazily loaded
- staff_context = {'definition': json.dumps(dict(module.definition), indent=4),
- 'metadata': json.dumps(dict(module.metadata), indent=4),
+ staff_context = {'definition': dict(module.definition),
+ 'metadata': dict(module.metadata),
'element_id': module.location.html_id(),
'edit_link': edit_link,
'histogram': json.dumps(histogram),
From 009bd230667bb722988e6aec69972ada6121d57d Mon Sep 17 00:00:00 2001
From: Victor Shnayder
Date: Tue, 31 Jul 2012 17:13:49 -0400
Subject: [PATCH 047/124] save LazyLoadingDict in case we do want it later
---
common/lib/xmodule/lazy_dict.py | 58 +++++++++++++++++++++++
common/lib/xmodule/xmodule/xml_module.py | 60 ------------------------
2 files changed, 58 insertions(+), 60 deletions(-)
create mode 100644 common/lib/xmodule/lazy_dict.py
diff --git a/common/lib/xmodule/lazy_dict.py b/common/lib/xmodule/lazy_dict.py
new file mode 100644
index 0000000000..fa614843ec
--- /dev/null
+++ b/common/lib/xmodule/lazy_dict.py
@@ -0,0 +1,58 @@
+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
+
diff --git a/common/lib/xmodule/xmodule/xml_module.py b/common/lib/xmodule/xmodule/xml_module.py
index de62bc5d38..89f1dc4ed2 100644
--- a/common/lib/xmodule/xmodule/xml_module.py
+++ b/common/lib/xmodule/xmodule/xml_module.py
@@ -11,66 +11,6 @@ import os
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')
From 0b67d1c40118c3a584c32bfe5b32f0f8c0a187df Mon Sep 17 00:00:00 2001
From: Victor Shnayder
Date: Tue, 31 Jul 2012 17:20:04 -0400
Subject: [PATCH 048/124] Turn error_handlers into error_trackers
* simplify logic--tracker just tracks errors. Trackers should not raise,
and are not be responsible for logging.
* adapted code to use trackers.
* Started cleanup of error handling code:
- if need to add info and re-raise, just do that. No logging.
- if working around a problem, log and track as needed.
---
common/lib/xmodule/tests/test_import.py | 4 +-
.../lib/xmodule/xmodule/backcompat_module.py | 11 +++--
common/lib/xmodule/xmodule/error_module.py | 15 ++++---
common/lib/xmodule/xmodule/errorhandlers.py | 45 -------------------
common/lib/xmodule/xmodule/errortracker.py | 33 ++++++++++++++
common/lib/xmodule/xmodule/mako_module.py | 4 +-
.../lib/xmodule/xmodule/modulestore/mongo.py | 27 ++++++-----
common/lib/xmodule/xmodule/modulestore/xml.py | 20 ++++-----
common/lib/xmodule/xmodule/raw_module.py | 10 ++---
common/lib/xmodule/xmodule/x_module.py | 38 ++++++++--------
common/lib/xmodule/xmodule/xml_module.py | 29 +++++-------
.../management/commands/clean_xml.py | 23 ++--------
12 files changed, 117 insertions(+), 142 deletions(-)
delete mode 100644 common/lib/xmodule/xmodule/errorhandlers.py
create mode 100644 common/lib/xmodule/xmodule/errortracker.py
diff --git a/common/lib/xmodule/tests/test_import.py b/common/lib/xmodule/tests/test_import.py
index 497354a18e..34ace135a3 100644
--- a/common/lib/xmodule/tests/test_import.py
+++ b/common/lib/xmodule/tests/test_import.py
@@ -4,7 +4,7 @@ import unittest
from lxml import etree
from xmodule.x_module import XMLParsingSystem, XModuleDescriptor
-from xmodule.errorhandlers import ignore_errors_handler
+from xmodule.errortracker import null_error_tracker
from xmodule.modulestore import Location
class ImportTestCase(unittest.TestCase):
@@ -27,7 +27,7 @@ class ImportTestCase(unittest.TestCase):
raise Exception("Shouldn't be called")
system = XMLParsingSystem(load_item, resources_fs,
- ignore_errors_handler, process_xml)
+ null_error_tracker, process_xml)
system.render_template = render_template
return system
diff --git a/common/lib/xmodule/xmodule/backcompat_module.py b/common/lib/xmodule/xmodule/backcompat_module.py
index da0d6788e4..7ace342d1b 100644
--- a/common/lib/xmodule/xmodule/backcompat_module.py
+++ b/common/lib/xmodule/xmodule/backcompat_module.py
@@ -35,18 +35,21 @@ def process_includes(fn):
# insert new XML into tree in place of include
parent.insert(parent.index(next_include), incxml)
except Exception:
- msg = "Error in problem xml include: %s" % (etree.tostring(next_include, pretty_print=True))
- log.exception(msg)
- parent = next_include.getparent()
+ # Log error and work around it
+ msg = "Error in problem xml include: %s" % (
+ etree.tostring(next_include, pretty_print=True))
+ system.error_tracker(msg)
+
+ parent = next_include.getparent()
errorxml = etree.Element('error')
messagexml = etree.SubElement(errorxml, 'message')
messagexml.text = msg
stackxml = etree.SubElement(errorxml, 'stacktrace')
stackxml.text = traceback.format_exc()
-
# insert error XML in place of include
parent.insert(parent.index(next_include), errorxml)
+
parent.remove(next_include)
next_include = xml_object.find('include')
diff --git a/common/lib/xmodule/xmodule/error_module.py b/common/lib/xmodule/xmodule/error_module.py
index ee05e9c879..62b3b82a39 100644
--- a/common/lib/xmodule/xmodule/error_module.py
+++ b/common/lib/xmodule/xmodule/error_module.py
@@ -15,7 +15,7 @@ class ErrorModule(XModule):
TODO (vshnayder): proper style, divs, etc.
'''
if not self.system.is_staff:
- return self.system.render_template('module-error.html')
+ return self.system.render_template('module-error.html', {})
# staff get to see all the details
return self.system.render_template('module-error-staff.html', {
@@ -31,22 +31,27 @@ class ErrorDescriptor(EditingDescriptor):
module_class = ErrorModule
@classmethod
- def from_xml(cls, xml_data, system, org=None, course=None):
+ def from_xml(cls, xml_data, system, org=None, course=None, err=None):
'''Create an instance of this descriptor from the supplied data.
Does not try to parse the data--just stores it.
+
+ Takes an extra, optional, parameter--the error that caused an
+ issue.
'''
definition = {}
+ if err is not None:
+ definition['error'] = err
+
try:
# If this is already an error tag, don't want to re-wrap it.
xml_obj = etree.fromstring(xml_data)
if xml_obj.tag == 'error':
xml_data = xml_obj.text
except etree.XMLSyntaxError as err:
- # Save the error to display later
- definition['error'] = str(err)
-
+ # Save the error to display later--overrides other problems
+ definition['error'] = err
definition['data'] = xml_data
# TODO (vshnayder): Do we need a unique slug here? Just pick a random
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..8dd893e814
--- /dev/null
+++ b/common/lib/xmodule/xmodule/errortracker.py
@@ -0,0 +1,33 @@
+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 make_error_tracker():
+ '''Return a tuple (logger, error_list), where
+ the logger appends a tuple (message, exc_info=None)
+ to the error_list on every call.
+
+ error_list is a simple list. If the caller messes with it, info
+ will be lost.
+ '''
+ errors = []
+
+ def error_tracker(msg):
+ '''Log errors'''
+ exc_info = None
+ if in_exception_handler():
+ exc_info = sys.exc_info()
+
+ errors.append((msg, exc_info))
+
+ return (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/mako_module.py b/common/lib/xmodule/xmodule/mako_module.py
index 213e9077db..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,
+ def __init__(self, load_item, resources_fs, error_tracker,
render_template, **kwargs):
super(MakoDescriptorSystem, self).__init__(
- load_item, resources_fs, error_handler, **kwargs)
+ load_item, resources_fs, error_tracker, **kwargs)
self.render_template = render_template
diff --git a/common/lib/xmodule/xmodule/modulestore/mongo.py b/common/lib/xmodule/xmodule/modulestore/mongo.py
index 6a0bdad06a..76769b25b0 100644
--- a/common/lib/xmodule/xmodule/modulestore/mongo.py
+++ b/common/lib/xmodule/xmodule/modulestore/mongo.py
@@ -6,7 +6,7 @@ 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 mitxmako.shortcuts import render_to_string
@@ -26,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
@@ -38,13 +38,13 @@ class CachingDescriptorSystem(MakoDescriptorSystem):
resources_fs: a filesystem, as per MakoDescriptorSystem
- error_handler:
+ error_tracker:
render_template: a function for rendering templates, as per
MakoDescriptorSystem
"""
super(CachingDescriptorSystem, self).__init__(
- self.load_item, resources_fs, error_handler, render_template)
+ self.load_item, resources_fs, error_tracker, render_template)
self.modulestore = modulestore
self.module_data = module_data
self.default_class = default_class
@@ -79,7 +79,8 @@ class MongoModuleStore(ModuleStore):
"""
# TODO (cpennington): Enable non-filesystem filestores
- def __init__(self, host, db, collection, fs_root, port=27017, default_class=None):
+ def __init__(self, host, db, collection, fs_root, port=27017, default_class=None,
+ error_tracker=null_error_tracker):
self.collection = pymongo.connection.Connection(
host=host,
port=port
@@ -90,13 +91,17 @@ class MongoModuleStore(ModuleStore):
# Force mongo to maintain an index over _id.* that is in the same order
# that is used when querying by a location
- self.collection.ensure_index(zip(('_id.' + field for field in Location._fields), repeat(1)))
+ self.collection.ensure_index(
+ zip(('_id.' + field for field in Location._fields), repeat(1)))
- # TODO (vshnayder): default arg default_class=None will make this error
- module_path, _, class_name = default_class.rpartition('.')
- class_ = getattr(import_module(module_path), class_name)
- self.default_class = class_
+ if default_class is not None:
+ module_path, _, class_name = default_class.rpartition('.')
+ class_ = getattr(import_module(module_path), class_name)
+ self.default_class = class_
+ else:
+ self.default_class = None
self.fs_root = path(fs_root)
+ self.error_tracker = error_tracker
def _clean_item_data(self, item):
"""
@@ -148,7 +153,7 @@ class MongoModuleStore(ModuleStore):
data_cache,
self.default_class,
resource_fs,
- strict_error_handler,
+ self.error_tracker,
render_to_string,
)
return system.load_item(item['location'])
diff --git a/common/lib/xmodule/xmodule/modulestore/xml.py b/common/lib/xmodule/xmodule/modulestore/xml.py
index b54963b513..06e824e3d1 100644
--- a/common/lib/xmodule/xmodule/modulestore/xml.py
+++ b/common/lib/xmodule/xmodule/modulestore/xml.py
@@ -3,7 +3,7 @@ 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 null_error_tracker
from xmodule.x_module import XModuleDescriptor, XMLParsingSystem
from xmodule.mako_module import MakoDescriptorSystem
from cStringIO import StringIO
@@ -29,7 +29,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, **kwargs):
+ 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.
@@ -89,9 +89,9 @@ class ImportSystem(XMLParsingSystem, MakoDescriptorSystem):
resources_fs = OSFS(xmlstore.data_dir / course_dir)
MakoDescriptorSystem.__init__(self, load_item, resources_fs,
- error_handler, render_template, **kwargs)
+ error_tracker, render_template, **kwargs)
XMLParsingSystem.__init__(self, load_item, resources_fs,
- error_handler, process_xml, **kwargs)
+ error_tracker, process_xml, **kwargs)
@@ -101,7 +101,7 @@ class XMLModuleStore(ModuleStore):
"""
def __init__(self, data_dir, default_class=None, eager=False,
course_dirs=None,
- error_handler=logging_error_handler):
+ error_tracker=null_error_tracker):
"""
Initialize an XMLModuleStore from data_dir
@@ -116,8 +116,8 @@ 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.
+ error_tracker: The error tracker used here and in the underlying
+ DescriptorSystem. By default, ignore all messages.
See the comments in x_module.py:DescriptorSystem
"""
@@ -125,7 +125,7 @@ class XMLModuleStore(ModuleStore):
self.data_dir = path(data_dir)
self.modules = {} # location -> XModuleDescriptor
self.courses = {} # course_dir -> XModuleDescriptor for the course
- self.error_handler = error_handler
+ self.error_tracker = error_tracker
if default_class is None:
self.default_class = None
@@ -154,7 +154,7 @@ class XMLModuleStore(ModuleStore):
except:
msg = "Failed to load course '%s'" % course_dir
log.exception(msg)
- error_handler(msg)
+ error_tracker(msg)
def load_course(self, course_dir):
@@ -192,7 +192,7 @@ class XMLModuleStore(ModuleStore):
course = course_dir
system = ImportSystem(self, org, course, course_dir,
- self.error_handler)
+ self.error_tracker)
course_descriptor = system.process_xml(etree.tostring(course_data))
log.debug('========> Done with course import from {0}'.format(course_dir))
diff --git a/common/lib/xmodule/xmodule/raw_module.py b/common/lib/xmodule/xmodule/raw_module.py
index f9f358f945..9fdb5d0b38 100644
--- a/common/lib/xmodule/xmodule/raw_module.py
+++ b/common/lib/xmodule/xmodule/raw_module.py
@@ -1,9 +1,8 @@
-from pkg_resources import resource_string
from lxml import etree
from xmodule.editing_module import EditingDescriptor
-from xmodule.mako_module import MakoModuleDescriptor
from xmodule.xml_module import XmlDescriptor
import logging
+import sys
log = logging.getLogger(__name__)
@@ -20,13 +19,12 @@ class RawDescriptor(XmlDescriptor, EditingDescriptor):
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/x_module.py b/common/lib/xmodule/xmodule/x_module.py
index 2d7447b5f6..a14f83eee6 100644
--- a/common/lib/xmodule/xmodule/x_module.py
+++ b/common/lib/xmodule/xmodule/x_module.py
@@ -454,15 +454,15 @@ class XModuleDescriptor(Plugin, HTMLSnippet):
# etree.fromstring(xml_data).tag,class_))
descriptor = class_.from_xml(xml_data, system, org, course)
- except (ResourceNotFoundError, XMLSyntaxError) as err:
+ 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
- #system.error_handler("Error loading from xml.")
- descriptor = ErrorDescriptor.from_xml(xml_data, system, org, course)
+ system.error_tracker("Error loading from xml.")
+ descriptor = ErrorDescriptor.from_xml(xml_data, system, org, course, err)
return descriptor
@@ -533,16 +533,19 @@ class XModuleDescriptor(Plugin, HTMLSnippet):
class DescriptorSystem(object):
- def __init__(self, load_item, resources_fs, error_handler, **kwargs):
+ 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:
@@ -551,10 +554,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:
@@ -562,25 +563,26 @@ 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, **kwargs):
+ 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
diff --git a/common/lib/xmodule/xmodule/xml_module.py b/common/lib/xmodule/xmodule/xml_module.py
index 89f1dc4ed2..ea13bc2640 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,13 +7,12 @@ import traceback
from collections import namedtuple
from fs.errors import ResourceNotFoundError
import os
+import sys
log = logging.getLogger(__name__)
-
_AttrMapBase = namedtuple('_AttrMap', 'metadata_key to_metadata from_metadata')
-
class AttrMap(_AttrMapBase):
"""
A class that specifies a metadata_key, and two functions:
@@ -116,11 +114,12 @@ class XmlDescriptor(XModuleDescriptor):
# 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'):
+ # 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):
@@ -130,19 +129,11 @@ class XmlDescriptor(XModuleDescriptor):
try:
with system.resources_fs.open(filepath) as file:
definition_xml = cls.file_to_xml(file)
- except (ResourceNotFoundError, etree.XMLSyntaxError):
+ except Exception:
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)}
+ # 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)
diff --git a/lms/djangoapps/courseware/management/commands/clean_xml.py b/lms/djangoapps/courseware/management/commands/clean_xml.py
index 05865bb289..29ce246637 100644
--- a/lms/djangoapps/courseware/management/commands/clean_xml.py
+++ b/lms/djangoapps/courseware/management/commands/clean_xml.py
@@ -10,7 +10,7 @@ from lxml import etree
from django.core.management.base import BaseCommand
from xmodule.modulestore.xml import XMLModuleStore
-
+from xmodule.errortracker import make_error_tracker
def traverse_tree(course):
'''Load every descriptor in course. Return bool success value.'''
@@ -21,23 +21,6 @@ def traverse_tree(course):
return True
-def make_logging_error_handler():
- '''Return a tuple (handler, error_list), where
- the handler appends the message and any exc_info
- to the error_list on every call.
- '''
- errors = []
-
- def error_handler(msg, exc_info=None):
- '''Log errors'''
- if exc_info is None:
- if sys.exc_info() != (None, None, None):
- exc_info = sys.exc_info()
-
- errors.append((msg, exc_info))
-
- return (error_handler, errors)
-
def export(course, export_dir):
"""Export the specified course to course_dir. Creates dir if it doesn't exist.
@@ -70,14 +53,14 @@ def import_with_checks(course_dir, verbose=True):
data_dir = course_dir.dirname()
course_dirs = [course_dir.basename()]
- (error_handler, errors) = make_logging_error_handler()
+ (error_tracker, errors) = make_error_tracker()
# No default class--want to complain if it doesn't find plugins for any
# module.
modulestore = XMLModuleStore(data_dir,
default_class=None,
eager=True,
course_dirs=course_dirs,
- error_handler=error_handler)
+ error_tracker=error_tracker)
def str_of_err(tpl):
(msg, exc_info) = tpl
From 740c9b7df1b51dcafd5e4babc132ce894aae2411 Mon Sep 17 00:00:00 2001
From: Victor Shnayder
Date: Tue, 31 Jul 2012 17:20:18 -0400
Subject: [PATCH 049/124] fixed docstring for customtag
---
common/lib/xmodule/xmodule/template_module.py | 6 ++----
1 file changed, 2 insertions(+), 4 deletions(-)
diff --git a/common/lib/xmodule/xmodule/template_module.py b/common/lib/xmodule/xmodule/template_module.py
index 3f926555f4..b4818dc29d 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
-
- $tagname
-
+
In this case, $tagname should refer to a file in data/custom_tags, which contains
a mako template that uses ${option} and ${option2} for the content.
For instance:
- data/custom_tags/book::
+ data/mycourse/custom_tags/book::
More information given in the text
course.xml::
From 463b758434eae2a33d9fb82f992cb139b25c50ea Mon Sep 17 00:00:00 2001
From: Victor Shnayder
Date: Tue, 31 Jul 2012 17:20:33 -0400
Subject: [PATCH 050/124] fixed args in call to add_histogram
---
lms/djangoapps/courseware/module_render.py | 2 +-
1 file changed, 1 insertion(+), 1 deletion(-)
diff --git a/lms/djangoapps/courseware/module_render.py b/lms/djangoapps/courseware/module_render.py
index 1ff39f602e..91d4efa651 100644
--- a/lms/djangoapps/courseware/module_render.py
+++ b/lms/djangoapps/courseware/module_render.py
@@ -185,7 +185,7 @@ def get_module(user, request, location, student_module_cache, position=None):
)
if settings.MITX_FEATURES.get('DISPLAY_HISTOGRAMS_TO_STAFF') and user.is_staff:
- module.get_html = add_histogram(module.get_html)
+ module.get_html = add_histogram(module.get_html, module)
# If StudentModule for this instance wasn't already in the database,
# and this isn't a guest user, create it.
From 32253510d1892a14357a766aa97e23a9cbc3da73 Mon Sep 17 00:00:00 2001
From: Victor Shnayder
Date: Tue, 31 Jul 2012 19:37:53 -0400
Subject: [PATCH 051/124] Import error cleanup
* call error tracker when needed
* remove duplicate logging--just add info and re-raise
* xml modulestore uses error tracker to capture load errors
* add unstyled list of import errors to courseware homepage!
---
.../lib/xmodule/xmodule/backcompat_module.py | 5 +-
common/lib/xmodule/xmodule/capa_module.py | 25 ++++++---
common/lib/xmodule/xmodule/course_module.py | 14 +++--
common/lib/xmodule/xmodule/errortracker.py | 10 ++--
common/lib/xmodule/xmodule/html_module.py | 5 +-
.../xmodule/xmodule/modulestore/__init__.py | 13 +++++
common/lib/xmodule/xmodule/modulestore/xml.py | 54 ++++++++++++-------
common/lib/xmodule/xmodule/x_module.py | 5 +-
lms/djangoapps/courseware/courses.py | 1 +
lms/djangoapps/courseware/views.py | 15 ++++--
lms/templates/courseware.html | 7 +++
11 files changed, 115 insertions(+), 39 deletions(-)
diff --git a/common/lib/xmodule/xmodule/backcompat_module.py b/common/lib/xmodule/xmodule/backcompat_module.py
index 7ace342d1b..c49f23b99e 100644
--- a/common/lib/xmodule/xmodule/backcompat_module.py
+++ b/common/lib/xmodule/xmodule/backcompat_module.py
@@ -35,12 +35,13 @@ def process_includes(fn):
# insert new XML into tree in place of include
parent.insert(parent.index(next_include), incxml)
except Exception:
- # Log error and work around it
+ # Log error
msg = "Error in problem xml include: %s" % (
etree.tostring(next_include, pretty_print=True))
-
+ # tell the tracker
system.error_tracker(msg)
+ # work around
parent = next_include.getparent()
errorxml = etree.Element('error')
messagexml = etree.SubElement(errorxml, 'message')
diff --git a/common/lib/xmodule/xmodule/capa_module.py b/common/lib/xmodule/xmodule/capa_module.py
index 2fae8b94e2..eed2cf3ac7 100644
--- a/common/lib/xmodule/xmodule/capa_module.py
+++ b/common/lib/xmodule/xmodule/capa_module.py
@@ -5,6 +5,7 @@ import json
import logging
import traceback
import re
+import sys
from datetime import timedelta
from lxml import etree
@@ -91,7 +92,8 @@ class CapaModule(XModule):
display_due_date_string = self.metadata.get('due', None)
if display_due_date_string is not None:
self.display_due_date = dateutil.parser.parse(display_due_date_string)
- #log.debug("Parsed " + display_due_date_string + " to " + str(self.display_due_date))
+ #log.debug("Parsed " + display_due_date_string +
+ # " to " + str(self.display_due_date))
else:
self.display_due_date = None
@@ -99,7 +101,8 @@ class CapaModule(XModule):
if grace_period_string is not None and self.display_due_date:
self.grace_period = parse_timedelta(grace_period_string)
self.close_date = self.display_due_date + self.grace_period
- #log.debug("Then parsed " + grace_period_string + " to closing date" + str(self.close_date))
+ #log.debug("Then parsed " + grace_period_string +
+ # " to closing date" + str(self.close_date))
else:
self.grace_period = None
self.close_date = self.display_due_date
@@ -138,10 +141,16 @@ class CapaModule(XModule):
try:
self.lcp = LoncapaProblem(self.definition['data'], self.location.html_id(),
instance_state, seed=seed, system=self.system)
- except Exception:
- msg = 'cannot create LoncapaProblem %s' % self.location.url()
- log.exception(msg)
+ except Exception as err:
+ msg = 'cannot create LoncapaProblem {loc}: {err}'.format(
+ loc=self.location.url(), err=err)
+ # TODO (vshnayder): do modules need error handlers too?
+ # We shouldn't be switching on DEBUG.
if self.system.DEBUG:
+ log.error(msg)
+ # TODO (vshnayder): This logic should be general, not here--and may
+ # want to preserve the data instead of replacing it.
+ # e.g. in the CMS
msg = '
%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/errortracker.py b/common/lib/xmodule/xmodule/errortracker.py
index 8dd893e814..09c04c061c 100644
--- a/common/lib/xmodule/xmodule/errortracker.py
+++ b/common/lib/xmodule/xmodule/errortracker.py
@@ -1,17 +1,21 @@
import logging
import sys
+from collections import namedtuple
+
log = logging.getLogger(__name__)
+ErrorLog = namedtuple('ErrorLog', 'tracker errors')
+
def in_exception_handler():
'''Is there an active exception?'''
return sys.exc_info() != (None, None, None)
def make_error_tracker():
- '''Return a tuple (logger, error_list), where
+ '''Return an ErrorLog (named tuple), with fields (tracker, errors), where
the logger appends a tuple (message, exc_info=None)
- to the error_list on every call.
+ to the errors on every call.
error_list is a simple list. If the caller messes with it, info
will be lost.
@@ -26,7 +30,7 @@ def make_error_tracker():
errors.append((msg, exc_info))
- return (error_tracker, errors)
+ return ErrorLog(error_tracker, errors)
def null_error_tracker(msg):
'''A dummy error tracker that just ignores the messages'''
diff --git a/common/lib/xmodule/xmodule/html_module.py b/common/lib/xmodule/xmodule/html_module.py
index 996d3c4ead..165c402487 100644
--- a/common/lib/xmodule/xmodule/html_module.py
+++ b/common/lib/xmodule/xmodule/html_module.py
@@ -2,6 +2,7 @@ import copy
from fs.errors import ResourceNotFoundError
import logging
import os
+import sys
from lxml import etree
from xmodule.x_module import XModule
@@ -95,8 +96,8 @@ class HtmlDescriptor(XmlDescriptor, EditingDescriptor):
except (ResourceNotFoundError) as err:
msg = 'Unable to load file contents at path {0}: {1} '.format(
filepath, err)
- log.error(msg)
- raise
+ # add more info and re-raise
+ raise Exception(msg), None, sys.exc_info()[2]
@classmethod
def split_to_file(cls, xml_object):
diff --git a/common/lib/xmodule/xmodule/modulestore/__init__.py b/common/lib/xmodule/xmodule/modulestore/__init__.py
index a0167d781b..7241179d8e 100644
--- a/common/lib/xmodule/xmodule/modulestore/__init__.py
+++ b/common/lib/xmodule/xmodule/modulestore/__init__.py
@@ -213,6 +213,18 @@ class ModuleStore(object):
"""
raise NotImplementedError
+ def get_item_errors(self, location):
+ """
+ Return a list of (msg, exception-or-None) errors that the modulestore
+ encountered when loading the item at location.
+
+ location : something that can be passed to Location
+
+ Raises the same exceptions as get_item if the location isn't found or
+ isn't fully specified.
+ """
+ raise NotImplementedError
+
def get_items(self, location, depth=0):
"""
Returns a list of XModuleDescriptor instances for the items
@@ -269,6 +281,7 @@ class ModuleStore(object):
'''
raise NotImplementedError
+
def get_parent_locations(self, location):
'''Find all locations that are the parents of this location. Needed
for path_to_location().
diff --git a/common/lib/xmodule/xmodule/modulestore/xml.py b/common/lib/xmodule/xmodule/modulestore/xml.py
index 06e824e3d1..e7f9c9ce0a 100644
--- a/common/lib/xmodule/xmodule/modulestore/xml.py
+++ b/common/lib/xmodule/xmodule/modulestore/xml.py
@@ -1,14 +1,16 @@
import logging
+import os
+import re
+
from fs.osfs import OSFS
from importlib import import_module
from lxml import etree
from path import path
-from xmodule.errortracker import null_error_tracker
+from xmodule.errortracker import ErrorLog, make_error_tracker
from xmodule.x_module import XModuleDescriptor, XMLParsingSystem
+from xmodule.course_module import CourseDescriptor
from xmodule.mako_module import MakoDescriptorSystem
from cStringIO import StringIO
-import os
-import re
from . import ModuleStore, Location
from .exceptions import ItemNotFoundError
@@ -19,7 +21,6 @@ etree.set_default_parser(
log = logging.getLogger('mitx.' + __name__)
-
# VS[compat]
# TODO (cpennington): Remove this once all fall 2012 courses have been imported
# into the cms from xml
@@ -94,14 +95,12 @@ class ImportSystem(XMLParsingSystem, MakoDescriptorSystem):
error_tracker, process_xml, **kwargs)
-
class XMLModuleStore(ModuleStore):
"""
An XML backed ModuleStore
"""
def __init__(self, data_dir, default_class=None, eager=False,
- course_dirs=None,
- error_tracker=null_error_tracker):
+ course_dirs=None):
"""
Initialize an XMLModuleStore from data_dir
@@ -115,17 +114,14 @@ class XMLModuleStore(ModuleStore):
course_dirs: If specified, the list of course_dirs to load. Otherwise,
load all course dirs
-
- error_tracker: The error tracker used here and in the underlying
- DescriptorSystem. By default, ignore all messages.
- See the comments in x_module.py:DescriptorSystem
"""
self.eager = eager
self.data_dir = path(data_dir)
self.modules = {} # location -> XModuleDescriptor
self.courses = {} # course_dir -> XModuleDescriptor for the course
- self.error_tracker = error_tracker
+ self.location_errors = {} # location -> ErrorLog
+
if default_class is None:
self.default_class = None
@@ -149,15 +145,18 @@ class XMLModuleStore(ModuleStore):
for course_dir in course_dirs:
try:
- course_descriptor = self.load_course(course_dir)
+ # make a tracker, then stick in the right place once the course loads
+ # and we know its location
+ errorlog = make_error_tracker()
+ course_descriptor = self.load_course(course_dir, errorlog.tracker)
self.courses[course_dir] = course_descriptor
+ self.location_errors[course_descriptor.location] = errorlog
except:
msg = "Failed to load course '%s'" % course_dir
log.exception(msg)
- error_tracker(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
@@ -191,13 +190,13 @@ class XMLModuleStore(ModuleStore):
))
course = course_dir
- system = ImportSystem(self, org, course, course_dir,
- self.error_tracker)
+ 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.
@@ -218,9 +217,28 @@ class XMLModuleStore(ModuleStore):
except KeyError:
raise ItemNotFoundError(location)
+
+ def get_item_errors(self, location):
+ """
+ Return list of errors for this location, if any. Raise the same
+ errors as get_item if location isn't present.
+
+ NOTE: This only actually works for courses in the xml datastore--
+ will return an empty list for all other modules.
+ """
+ location = Location(location)
+ # check that item is present
+ self.get_item(location)
+
+ # now look up errors
+ if location in self.location_errors:
+ return self.location_errors[location].errors
+ return []
+
def get_courses(self, depth=0):
"""
- Returns a list of course descriptors
+ Returns a list of course descriptors. If there were errors on loading,
+ some of these may be ErrorDescriptors instead.
"""
return self.courses.values()
diff --git a/common/lib/xmodule/xmodule/x_module.py b/common/lib/xmodule/xmodule/x_module.py
index a14f83eee6..2a1769bbd7 100644
--- a/common/lib/xmodule/xmodule/x_module.py
+++ b/common/lib/xmodule/xmodule/x_module.py
@@ -460,8 +460,9 @@ class XModuleDescriptor(Plugin, HTMLSnippet):
# Put import here to avoid circular import errors
from xmodule.error_module import ErrorDescriptor
-
- system.error_tracker("Error loading from xml.")
+ msg = "Error loading from xml."
+ log.exception(msg)
+ system.error_tracker(msg)
descriptor = ErrorDescriptor.from_xml(xml_data, system, org, course, err)
return descriptor
diff --git a/lms/djangoapps/courseware/courses.py b/lms/djangoapps/courseware/courses.py
index c2c391b08e..cfda6497d5 100644
--- a/lms/djangoapps/courseware/courses.py
+++ b/lms/djangoapps/courseware/courses.py
@@ -33,6 +33,7 @@ def check_course(course_id, course_must_be_open=True, course_required=True):
try:
course_loc = CourseDescriptor.id_to_location(course_id)
course = modulestore().get_item(course_loc)
+
except (KeyError, ItemNotFoundError):
raise Http404("Course not found.")
diff --git a/lms/djangoapps/courseware/views.py b/lms/djangoapps/courseware/views.py
index fb4d2942f9..a049638d1b 100644
--- a/lms/djangoapps/courseware/views.py
+++ b/lms/djangoapps/courseware/views.py
@@ -55,14 +55,20 @@ def user_groups(user):
def format_url_params(params):
- return [urllib.quote(string.replace(' ', '_')) for string in params]
+ return [urllib.quote(string.replace(' ', '_'))
+ if string is not None else None
+ for string in params]
@ensure_csrf_cookie
@cache_if_anonymous
def courses(request):
# TODO: Clean up how 'error' is done.
- courses = sorted(modulestore().get_courses(), key=lambda course: course.number)
+
+ # filter out any courses that errored.
+ courses = [c for c in modulestore().get_courses()
+ if isinstance(c, CourseDescriptor)]
+ courses = sorted(courses, key=lambda course: course.number)
universities = defaultdict(list)
for course in courses:
universities[course.org].append(course)
@@ -210,7 +216,10 @@ def index(request, course_id, chapter=None, section=None,
log.warning("Couldn't find a section descriptor for course_id '{0}',"
"chapter '{1}', section '{2}'".format(
course_id, chapter, section))
-
+ else:
+ if request.user.is_staff:
+ # Add a list of all the errors...
+ context['course_errors'] = modulestore().get_item_errors(course.location)
result = render_to_response('courseware.html', context)
except:
diff --git a/lms/templates/courseware.html b/lms/templates/courseware.html
index 9c145ba8c0..0955134694 100644
--- a/lms/templates/courseware.html
+++ b/lms/templates/courseware.html
@@ -47,6 +47,13 @@
${content}
+
+ % if course_errors is not UNDEFINED:
+
Course errors
+
+ ${course_errors}
+
+ % endif
From 05c22c4901158bdca2c23242473aacd0b2cf6325 Mon Sep 17 00:00:00 2001
From: Victor Shnayder
Date: Wed, 1 Aug 2012 09:29:31 -0400
Subject: [PATCH 052/124] Prettier error display
* Log formatted traceback string instead of exc_info tuple itself
* display as a list
---
common/lib/xmodule/xmodule/errortracker.py | 13 +++++++------
lms/templates/courseware.html | 8 +++++++-
2 files changed, 14 insertions(+), 7 deletions(-)
diff --git a/common/lib/xmodule/xmodule/errortracker.py b/common/lib/xmodule/xmodule/errortracker.py
index 09c04c061c..b8d42a6983 100644
--- a/common/lib/xmodule/xmodule/errortracker.py
+++ b/common/lib/xmodule/xmodule/errortracker.py
@@ -1,5 +1,6 @@
import logging
import sys
+import traceback
from collections import namedtuple
@@ -14,21 +15,21 @@ def in_exception_handler():
def make_error_tracker():
'''Return an ErrorLog (named tuple), with fields (tracker, errors), where
- the logger appends a tuple (message, exc_info=None)
- to the errors on every call.
+ 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 messes with it, info
+ error_list is a simple list. If the caller modifies it, info
will be lost.
'''
errors = []
def error_tracker(msg):
'''Log errors'''
- exc_info = None
+ exc_str = ''
if in_exception_handler():
- exc_info = sys.exc_info()
+ exc_str = ''.join(traceback.format_exception(*sys.exc_info()))
- errors.append((msg, exc_info))
+ errors.append((msg, exc_str))
return ErrorLog(error_tracker, errors)
diff --git a/lms/templates/courseware.html b/lms/templates/courseware.html
index 0955134694..a14f35d154 100644
--- a/lms/templates/courseware.html
+++ b/lms/templates/courseware.html
@@ -51,7 +51,13 @@
% if course_errors is not UNDEFINED:
Course errors
- ${course_errors}
+
+ % for (msg, err) in course_errors:
+
${msg}
+
${err}
+
+ % endfor
+
% endif
From 7fb831a2e8f398e4cf1b9863b26627d6e5777a0e Mon Sep 17 00:00:00 2001
From: Victor Shnayder
Date: Wed, 1 Aug 2012 09:36:13 -0400
Subject: [PATCH 053/124] Record warning on bad html files
---
common/lib/xmodule/xmodule/html_module.py | 4 +++-
1 file changed, 3 insertions(+), 1 deletion(-)
diff --git a/common/lib/xmodule/xmodule/html_module.py b/common/lib/xmodule/xmodule/html_module.py
index 165c402487..b2a5df9803 100644
--- a/common/lib/xmodule/xmodule/html_module.py
+++ b/common/lib/xmodule/xmodule/html_module.py
@@ -91,7 +91,9 @@ class HtmlDescriptor(XmlDescriptor, EditingDescriptor):
html = file.read()
# Log a warning if we can't parse the file, but don't error
if not check_html(html):
- log.warning("Couldn't parse html in {0}.".format(filepath))
+ 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(
From 10ebcefd5427a8083581a877990517d0485ee5b1 Mon Sep 17 00:00:00 2001
From: ichuang
Date: Wed, 1 Aug 2012 14:59:55 -0400
Subject: [PATCH 054/124] add openid authentication
---
common/djangoapps/student/views.py | 45 +++++-
lms/envs/dev.py | 12 ++
lms/envs/dev_ike.py | 222 ++++++++++++-----------------
lms/templates/extauth_failure.html | 11 ++
lms/templates/index.html | 9 ++
lms/templates/login_modal.html | 3 +
lms/templates/signup_modal.html | 15 ++
lms/urls.py | 12 +-
requirements.txt | 2 +
9 files changed, 199 insertions(+), 132 deletions(-)
create mode 100644 lms/templates/extauth_failure.html
diff --git a/common/djangoapps/student/views.py b/common/djangoapps/student/views.py
index b739e0e37c..692b135ff6 100644
--- a/common/djangoapps/student/views.py
+++ b/common/djangoapps/student/views.py
@@ -61,6 +61,15 @@ def index(request):
if settings.COURSEWARE_ENABLED and request.user.is_authenticated():
return redirect(reverse('dashboard'))
+ return main_index()
+
+def main_index(extra_context = {}):
+ '''
+ Render the edX main page.
+
+ extra_context is used to allow immediate display of certain modal windows, eg signup,
+ as used by external_auth.
+ '''
feed_data = cache.get("students_index_rss_feed_data")
if feed_data == None:
if hasattr(settings, 'RSS_URL'):
@@ -81,8 +90,9 @@ def index(request):
for course in courses:
universities[course.org].append(course)
- return render_to_response('index.html', {'universities': universities, 'entries': entries})
-
+ context = {'universities': universities, 'entries': entries}
+ context.update(extra_context)
+ return render_to_response('index.html', context)
def course_from_id(id):
course_loc = CourseDescriptor.id_to_location(id)
@@ -257,11 +267,26 @@ def change_setting(request):
@ensure_csrf_cookie
def create_account(request, post_override=None):
- ''' JSON call to enroll in the course. '''
+ '''
+ JSON call to create new edX account.
+ Used by form in signup_modal.html, which is included into navigation.html
+ '''
js = {'success': False}
post_vars = post_override if post_override else request.POST
+ # if doing signup for an external authorization, then get email, password, name from the eamap
+ # don't use the ones from the form, since the user could have hacked those
+ doExternalAuth = 'ExternalAuthMap' in request.session
+ if doExternalAuth:
+ eamap = request.session['ExternalAuthMap']
+ email = eamap.external_email
+ name = eamap.external_name
+ password = eamap.internal_password
+ post_vars = dict(post_vars.items())
+ post_vars.update(dict(email=email, name=name, password=password, username=post_vars['username']))
+ log.debug('extauth test: post_vars = %s' % post_vars)
+
# Confirm we have a properly formed request
for a in ['username', 'email', 'password', 'name']:
if a not in post_vars:
@@ -356,8 +381,9 @@ def create_account(request, post_override=None):
'key': r.activation_key,
}
+ # composes activation email
subject = render_to_string('emails/activation_email_subject.txt', d)
- # Email subject *must not* contain newlines
+ # Email subject *must not* contain newlines
subject = ''.join(subject.splitlines())
message = render_to_string('emails/activation_email.txt', d)
@@ -382,6 +408,17 @@ def create_account(request, post_override=None):
try_change_enrollment(request)
+ if doExternalAuth:
+ eamap.user = login_user
+ eamap.dtsignup = datetime.datetime.now()
+ eamap.save()
+ log.debug('Updated ExternalAuthMap for %s to be %s' % (post_vars['username'],eamap))
+
+ if settings.MITX_FEATURES.get('BYPASS_ACTIVATION_EMAIL_FOR_EXTAUTH'):
+ log.debug('bypassing activation email')
+ login_user.is_active = True
+ login_user.save()
+
js = {'success': True}
return HttpResponse(json.dumps(js), mimetype="application/json")
diff --git a/lms/envs/dev.py b/lms/envs/dev.py
index 1a2659cb1f..f9b7ba10a0 100644
--- a/lms/envs/dev.py
+++ b/lms/envs/dev.py
@@ -55,6 +55,18 @@ CACHES = {
# Dummy secret key for dev
SECRET_KEY = '85920908f28904ed733fe576320db18cabd7b6cd'
+################################ OpenID Auth #################################
+MITX_FEATURES['AUTH_USE_OPENID'] = True
+MITX_FEATURES['BYPASS_ACTIVATION_EMAIL_FOR_EXTAUTH'] = True
+
+INSTALLED_APPS += ('external_auth',)
+INSTALLED_APPS += ('django_openid_auth',)
+
+OPENID_CREATE_USERS = False
+OPENID_UPDATE_DETAILS_FROM_SREG = True
+OPENID_SSO_SERVER_URL = 'https://www.google.com/accounts/o8/id' # TODO: accept more endpoints
+OPENID_USE_AS_ADMIN_LOGIN = False
+
################################ DEBUG TOOLBAR #################################
INSTALLED_APPS += ('debug_toolbar',)
MIDDLEWARE_CLASSES += ('debug_toolbar.middleware.DebugToolbarMiddleware',)
diff --git a/lms/envs/dev_ike.py b/lms/envs/dev_ike.py
index 79ae3354ac..fb7d980550 100644
--- a/lms/envs/dev_ike.py
+++ b/lms/envs/dev_ike.py
@@ -7,142 +7,110 @@ sessions. Assumes structure:
/mitx # The location of this repo
/log # Where we're going to write log files
"""
-
-import socket
-
-if 'eecs1' in socket.gethostname():
- MITX_ROOT_URL = '/mitx2'
-
from .common import *
from .logsettings import get_logger_config
-from .dev import *
-
-if 'eecs1' in socket.gethostname():
- # MITX_ROOT_URL = '/mitx2'
- MITX_ROOT_URL = 'https://eecs1.mit.edu/mitx2'
-
-#-----------------------------------------------------------------------------
-# edx4edx content server
-
-EMAIL_BACKEND = 'django.core.mail.backends.smtp.EmailBackend'
-MITX_FEATURES['REROUTE_ACTIVATION_EMAIL'] = 'ichuang@mit.edu'
-EDX4EDX_ROOT = ENV_ROOT / "data/edx4edx"
-
-#EMAIL_BACKEND = 'django_ses.SESBackend'
-
-#-----------------------------------------------------------------------------
-# ichuang
DEBUG = True
-ENABLE_MULTICOURSE = True # set to False to disable multicourse display (see lib.util.views.mitxhome)
-QUICKEDIT = False
+TEMPLATE_DEBUG = True
-MAKO_TEMPLATES['course'] = [DATA_DIR, EDX4EDX_ROOT ]
+MITX_FEATURES['DISABLE_START_DATES'] = True
-#MITX_FEATURES['USE_DJANGO_PIPELINE'] = False
-MITX_FEATURES['DISPLAY_HISTOGRAMS_TO_STAFF'] = False
-MITX_FEATURES['DISPLAY_EDIT_LINK'] = True
-MITX_FEATURES['DEBUG_LEVEL'] = 10 # 0 = lowest level, least verbose, 255 = max level, most verbose
+WIKI_ENABLED = True
-COURSE_SETTINGS = {'6.002x_Fall_2012': {'number' : '6.002x',
- 'title' : 'Circuits and Electronics',
- 'xmlpath': '/6002x-fall-2012/',
- 'active' : True,
- 'default_chapter' : 'Week_1',
- 'default_section' : 'Administrivia_and_Circuit_Elements',
- 'location': 'i4x://edx/6002xs12/course/6.002x_Fall_2012',
- },
- '8.02_Spring_2013': {'number' : '8.02x',
- 'title' : 'Electricity & Magnetism',
- 'xmlpath': '/802x/',
- 'github_url': 'https://github.com/MITx/8.02x',
- 'active' : True,
- 'default_chapter' : 'Introduction',
- 'default_section' : 'Introduction_%28Lewin_2002%29',
- },
- '6.189_Spring_2013': {'number' : '6.189x',
- 'title' : 'IAP Python Programming',
- 'xmlpath': '/6.189x/',
- 'github_url': 'https://github.com/MITx/6.189x',
- 'active' : True,
- 'default_chapter' : 'Week_1',
- 'default_section' : 'Variables_and_Binding',
- },
- '8.01_Fall_2012': {'number' : '8.01x',
- 'title' : 'Mechanics',
- 'xmlpath': '/8.01x/',
- 'github_url': 'https://github.com/MITx/8.01x',
- 'active': True,
- 'default_chapter' : 'Mechanics_Online_Spring_2012',
- 'default_section' : 'Introduction_to_the_course',
- 'location': 'i4x://edx/6002xs12/course/8.01_Fall_2012',
- },
- 'edx4edx': {'number' : 'edX.01',
- 'title' : 'edx4edx: edX Author Course',
- 'xmlpath': '/edx4edx/',
- 'github_url': 'https://github.com/MITx/edx4edx',
- 'active' : True,
- 'default_chapter' : 'Introduction',
- 'default_section' : 'edx4edx_Course',
- 'location': 'i4x://edx/6002xs12/course/edx4edx',
- },
- '7.03x_Fall_2012': {'number' : '7.03x',
- 'title' : 'Genetics',
- 'xmlpath': '/7.03x/',
- 'github_url': 'https://github.com/MITx/7.03x',
- 'active' : True,
- 'default_chapter' : 'Week_2',
- 'default_section' : 'ps1_question_1',
- },
- '3.091x_Fall_2012': {'number' : '3.091x',
- 'title' : 'Introduction to Solid State Chemistry',
- 'xmlpath': '/3.091x/',
- 'github_url': 'https://github.com/MITx/3.091x',
- 'active' : True,
- 'default_chapter' : 'Week_1',
- 'default_section' : 'Problem_Set_1',
- },
- '18.06x_Linear_Algebra': {'number' : '18.06x',
- 'title' : 'Linear Algebra',
- 'xmlpath': '/18.06x/',
- 'github_url': 'https://github.com/MITx/18.06x',
- 'default_chapter' : 'Unit_1',
- 'default_section' : 'Midterm_1',
- 'active' : True,
- },
- '6.00x_Fall_2012': {'number' : '6.00x',
- 'title' : 'Introduction to Computer Science and Programming',
- 'xmlpath': '/6.00x/',
- 'github_url': 'https://github.com/MITx/6.00x',
- 'active' : True,
- 'default_chapter' : 'Week_0',
- 'default_section' : 'Problem_Set_0',
- 'location': 'i4x://edx/6002xs12/course/6.00x_Fall_2012',
- },
- '7.00x_Fall_2012': {'number' : '7.00x',
- 'title' : 'Introduction to Biology',
- 'xmlpath': '/7.00x/',
- 'github_url': 'https://github.com/MITx/7.00x',
- 'active' : True,
- 'default_chapter' : 'Unit 1',
- 'default_section' : 'Introduction',
- },
- }
+LOGGING = get_logger_config(ENV_ROOT / "log",
+ logging_env="dev",
+ tracking_filename="tracking.log",
+ debug=True)
-#-----------------------------------------------------------------------------
+DATABASES = {
+ 'default': {
+ 'ENGINE': 'django.db.backends.sqlite3',
+ 'NAME': ENV_ROOT / "db" / "mitx.db",
+ }
+}
-MIDDLEWARE_CLASSES = MIDDLEWARE_CLASSES + (
- 'ssl_auth.ssl_auth.NginxProxyHeaderMiddleware', # ssl authentication behind nginx proxy
- )
+CACHES = {
+ # This is the cache used for most things. Askbot will not work without a
+ # functioning cache -- it relies on caching to load its settings in places.
+ # In staging/prod envs, the sessions also live here.
+ 'default': {
+ 'BACKEND': 'django.core.cache.backends.locmem.LocMemCache',
+ 'LOCATION': 'mitx_loc_mem_cache',
+ 'KEY_FUNCTION': 'util.memcache.safe_key',
+ },
-AUTHENTICATION_BACKENDS = (
- 'ssl_auth.ssl_auth.SSLLoginBackend',
- 'django.contrib.auth.backends.ModelBackend',
- )
+ # The general cache is what you get if you use our util.cache. It's used for
+ # things like caching the course.xml file for different A/B test groups.
+ # We set it to be a DummyCache to force reloading of course.xml in dev.
+ # In staging environments, we would grab VERSION from data uploaded by the
+ # push process.
+ 'general': {
+ 'BACKEND': 'django.core.cache.backends.dummy.DummyCache',
+ 'KEY_PREFIX': 'general',
+ 'VERSION': 4,
+ 'KEY_FUNCTION': 'util.memcache.safe_key',
+ }
+}
-INSTALLED_APPS = INSTALLED_APPS + (
- 'ssl_auth',
- )
+# Dummy secret key for dev
+SECRET_KEY = '85920908f28904ed733fe576320db18cabd7b6cd'
-LOGIN_REDIRECT_URL = MITX_ROOT_URL + '/'
-LOGIN_URL = MITX_ROOT_URL + '/'
+################################ OpenID Auth #################################
+MITX_FEATURES['AUTH_USE_OPENID'] = True
+
+INSTALLED_APPS += ('external_auth',)
+INSTALLED_APPS += ('django_openid_auth',)
+#INSTALLED_APPS += ('ssl_auth',)
+
+#MIDDLEWARE_CLASSES += (
+# #'ssl_auth.ssl_auth.NginxProxyHeaderMiddleware', # ssl authentication behind nginx proxy
+# )
+
+#AUTHENTICATION_BACKENDS = (
+# 'django_openid_auth.auth.OpenIDBackend',
+# 'django.contrib.auth.backends.ModelBackend',
+# )
+
+OPENID_CREATE_USERS = False
+OPENID_UPDATE_DETAILS_FROM_SREG = True
+OPENID_SSO_SERVER_URL = 'https://www.google.com/accounts/o8/id'
+OPENID_USE_AS_ADMIN_LOGIN = False
+#import external_auth.views as edXauth
+#OPENID_RENDER_FAILURE = edXauth.edXauth_openid
+
+################################ DEBUG TOOLBAR #################################
+INSTALLED_APPS += ('debug_toolbar',)
+MIDDLEWARE_CLASSES += ('debug_toolbar.middleware.DebugToolbarMiddleware',)
+INTERNAL_IPS = ('127.0.0.1',)
+
+DEBUG_TOOLBAR_PANELS = (
+ 'debug_toolbar.panels.version.VersionDebugPanel',
+ 'debug_toolbar.panels.timer.TimerDebugPanel',
+ 'debug_toolbar.panels.settings_vars.SettingsVarsDebugPanel',
+ 'debug_toolbar.panels.headers.HeaderDebugPanel',
+ 'debug_toolbar.panels.request_vars.RequestVarsDebugPanel',
+ 'debug_toolbar.panels.sql.SQLDebugPanel',
+ 'debug_toolbar.panels.signals.SignalDebugPanel',
+ 'debug_toolbar.panels.logger.LoggingPanel',
+
+# Enabling the profiler has a weird bug as of django-debug-toolbar==0.9.4 and
+# Django=1.3.1/1.4 where requests to views get duplicated (your method gets
+# hit twice). So you can uncomment when you need to diagnose performance
+# problems, but you shouldn't leave it on.
+# 'debug_toolbar.panels.profiling.ProfilingDebugPanel',
+)
+
+############################ FILE UPLOADS (ASKBOT) #############################
+DEFAULT_FILE_STORAGE = 'django.core.files.storage.FileSystemStorage'
+MEDIA_ROOT = ENV_ROOT / "uploads"
+MEDIA_URL = "/static/uploads/"
+STATICFILES_DIRS.append(("uploads", MEDIA_ROOT))
+FILE_UPLOAD_TEMP_DIR = ENV_ROOT / "uploads"
+FILE_UPLOAD_HANDLERS = (
+ 'django.core.files.uploadhandler.MemoryFileUploadHandler',
+ 'django.core.files.uploadhandler.TemporaryFileUploadHandler',
+)
+
+########################### PIPELINE #################################
+
+PIPELINE_SASS_ARGUMENTS = '-r {proj_dir}/static/sass/bourbon/lib/bourbon.rb'.format(proj_dir=PROJECT_ROOT)
diff --git a/lms/templates/extauth_failure.html b/lms/templates/extauth_failure.html
new file mode 100644
index 0000000000..fa53ab1084
--- /dev/null
+++ b/lms/templates/extauth_failure.html
@@ -0,0 +1,11 @@
+
+
+
+ OpenID failed
+
+
+
From 102d4906d0d1c94aec6d2854e9e46c6707c0d09e Mon Sep 17 00:00:00 2001
From: ichuang
Date: Wed, 1 Aug 2012 22:41:27 -0400
Subject: [PATCH 069/124] fix problem with add_histogram (expects module as
second argument)
---
lms/djangoapps/courseware/module_render.py | 2 +-
1 file changed, 1 insertion(+), 1 deletion(-)
diff --git a/lms/djangoapps/courseware/module_render.py b/lms/djangoapps/courseware/module_render.py
index 80a4ef90fc..16921d1d50 100644
--- a/lms/djangoapps/courseware/module_render.py
+++ b/lms/djangoapps/courseware/module_render.py
@@ -184,7 +184,7 @@ def get_module(user, request, location, student_module_cache, position=None):
)
if settings.MITX_FEATURES.get('DISPLAY_HISTOGRAMS_TO_STAFF') and user.is_staff:
- module.get_html = add_histogram(module.get_html)
+ module.get_html = add_histogram(module.get_html, module)
# If StudentModule for this instance wasn't already in the database,
# and this isn't a guest user, create it.
From a759850e3e45a04e5953320fdb001e98df6a81be Mon Sep 17 00:00:00 2001
From: ichuang
Date: Wed, 1 Aug 2012 22:42:06 -0400
Subject: [PATCH 070/124] add SSL / MIT certificates auth; clean up
external_auth.views
---
common/djangoapps/external_auth/views.py | 162 +++++++++----
common/djangoapps/student/views.py | 4 +
lms/djangoapps/ssl_auth/__init__.py | 0
lms/djangoapps/ssl_auth/ssl_auth.py | 290 -----------------------
lms/envs/dev.py | 3 +
5 files changed, 124 insertions(+), 335 deletions(-)
delete mode 100644 lms/djangoapps/ssl_auth/__init__.py
delete mode 100755 lms/djangoapps/ssl_auth/ssl_auth.py
diff --git a/common/djangoapps/external_auth/views.py b/common/djangoapps/external_auth/views.py
index 57131f21da..5004d614d5 100644
--- a/common/djangoapps/external_auth/views.py
+++ b/common/djangoapps/external_auth/views.py
@@ -1,8 +1,7 @@
-# from pprint import pprint
-
import json
import logging
import random
+import re
import string
from external_auth.models import ExternalAuthMap
@@ -25,7 +24,6 @@ except ImportError:
from django_future.csrf import ensure_csrf_cookie
from util.cache import cache_if_anonymous
-#from django_openid_auth import views as openid_views
from django_openid_auth import auth as openid_auth
from openid.consumer.consumer import (Consumer, SUCCESS, CANCEL, FAILURE)
import django_openid_auth
@@ -43,6 +41,12 @@ def default_render_failure(request, message, status=403, template_name='extauth_
data = render_to_string( template_name, dict(message=message, exception=exception))
return HttpResponse(data, status=status)
+#-----------------------------------------------------------------------------
+# Openid
+
+def GenPasswd(length=12, chars=string.letters + string.digits):
+ return ''.join([random.choice(chars) for i in range(length)])
+
@csrf_exempt
def edXauth_openid_login_complete(request, redirect_field_name=REDIRECT_FIELD_NAME, render_failure=None):
"""Complete the openid login process"""
@@ -63,50 +67,62 @@ def edXauth_openid_login_complete(request, redirect_field_name=REDIRECT_FIELD_N
log.debug('openid success, details=%s' % details)
- # see if we have a map from this external_id to an edX username
- try:
- eamap = ExternalAuthMap.objects.get(external_id=external_id)
- log.debug('Found eamap=%s' % eamap)
- except ExternalAuthMap.DoesNotExist:
- # go render form for creating edX user
- eamap = ExternalAuthMap(external_id = external_id,
- external_domain = "openid:%s" % settings.OPENID_SSO_SERVER_URL,
- external_credentials = json.dumps(details),
- )
- eamap.external_email = details.get('email','')
- eamap.external_name = '%s %s' % (details.get('first_name',''),details.get('last_name',''))
-
- def GenPasswd(length=12, chars=string.letters + string.digits):
- return ''.join([random.choice(chars) for i in range(length)])
- eamap.internal_password = GenPasswd()
- log.debug('created eamap=%s' % eamap)
-
- eamap.save()
-
- internal_user = eamap.user
- if internal_user is None:
- log.debug('ExtAuth: no user for %s yet, doing signup' % eamap.external_email)
- return edXauth_signup(request, eamap)
-
- uname = internal_user.username
- user = authenticate(username=uname, password=eamap.internal_password)
- if user is None:
- log.warning("External Auth Login failed for %s / %s" % (uname,eamap.internal_password))
- return edXauth_signup(request, eamap)
-
- if not user.is_active:
- log.warning("External Auth: user %s is not active" % (uname))
- # TODO: improve error page
- return render_failure(request, 'Account not yet activated: please look for link in your email')
-
- login(request, user)
- request.session.set_expiry(0)
- student_views.try_change_enrollment(request)
- log.info("Login success - {0} ({1})".format(user.username, user.email))
- return redirect('/')
-
+ return edXauth_external_login_or_signup(request,
+ external_id,
+ "openid:%s" % settings.OPENID_SSO_SERVER_URL,
+ details,
+ details.get('email',''),
+ '%s %s' % (details.get('first_name',''),details.get('last_name',''))
+ )
+
return render_failure(request, 'Openid failure')
+
+#-----------------------------------------------------------------------------
+# generic external auth login or signup
+
+def edXauth_external_login_or_signup(request, external_id, external_domain, credentials, email, fullname):
+ # see if we have a map from this external_id to an edX username
+ try:
+ eamap = ExternalAuthMap.objects.get(external_id=external_id)
+ log.debug('Found eamap=%s' % eamap)
+ except ExternalAuthMap.DoesNotExist:
+ # go render form for creating edX user
+ eamap = ExternalAuthMap(external_id = external_id,
+ external_domain = external_domain,
+ external_credentials = json.dumps(credentials),
+ )
+ eamap.external_email = email
+ eamap.external_name = fullname
+ eamap.internal_password = GenPasswd()
+ log.debug('created eamap=%s' % eamap)
+
+ eamap.save()
+
+ internal_user = eamap.user
+ if internal_user is None:
+ log.debug('ExtAuth: no user for %s yet, doing signup' % eamap.external_email)
+ return edXauth_signup(request, eamap)
+ uname = internal_user.username
+ user = authenticate(username=uname, password=eamap.internal_password)
+ if user is None:
+ log.warning("External Auth Login failed for %s / %s" % (uname,eamap.internal_password))
+ return edXauth_signup(request, eamap)
+
+ if not user.is_active:
+ log.warning("External Auth: user %s is not active" % (uname))
+ # TODO: improve error page
+ return render_failure(request, 'Account not yet activated: please look for link in your email')
+
+ login(request, user)
+ request.session.set_expiry(0)
+ student_views.try_change_enrollment(request)
+ log.info("Login success - {0} ({1})".format(user.username, user.email))
+ return redirect('/')
+
+#-----------------------------------------------------------------------------
+# generic external auth signup
+
@ensure_csrf_cookie
@cache_if_anonymous
def edXauth_signup(request, eamap=None):
@@ -135,3 +151,59 @@ def edXauth_signup(request, eamap=None):
log.debug('ExtAuth: doing signup for %s' % eamap.external_email)
return student_views.main_index(extra_context=context)
+
+#-----------------------------------------------------------------------------
+# MIT SSL
+
+def ssl_dn_extract_info(dn):
+ '''
+ Extract username, email address (may be anyuser@anydomain.com) and full name
+ from the SSL DN string. Return (user,email,fullname) if successful, and None
+ otherwise.
+ '''
+ ss = re.search('/emailAddress=(.*)@([^/]+)', dn)
+ if ss:
+ user = ss.group(1)
+ email = "%s@%s" % (user, ss.group(2))
+ else:
+ return None
+ ss = re.search('/CN=([^/]+)/', dn)
+ if ss:
+ fullname = ss.group(1)
+ else:
+ return None
+ return (user, email, fullname)
+
+@csrf_exempt
+def edXauth_ssl_login(request):
+ """
+ This is called by student.views.index when MITX_FEATURES['AUTH_USE_MIT_CERTIFICATES'] = True
+
+ Used for MIT user authentication. This presumes the web server (nginx) has been configured
+ to require specific client certificates.
+
+ If the incoming protocol is HTTPS (SSL) then authenticate via client certificate.
+ The certificate provides user email and fullname; this populates the ExternalAuthMap.
+ The user is nevertheless still asked to complete the edX signup.
+
+ Else continues on with student.views.main_index, and no authentication.
+ """
+ certkey = "SSL_CLIENT_S_DN" # specify the request.META field to use
+
+ cert = request.META.get(certkey,'')
+ if not cert:
+ cert = request.META.get('HTTP_'+certkey,'')
+ if not cert:
+ cert = request._req.subprocess_env.get(certkey,'') # try the direct apache2 SSL key
+ if not cert:
+ # no certificate information - go onward to main index
+ return student_views.main_index()
+
+ (user, email, fullname) = ssl_dn_extract_info(cert)
+
+ return edXauth_external_login_or_signup(request,
+ external_id=email,
+ external_domain="ssl:MIT",
+ credentials=cert,
+ email=email,
+ fullname=fullname)
diff --git a/common/djangoapps/student/views.py b/common/djangoapps/student/views.py
index 3ba83f42bb..7937d67980 100644
--- a/common/djangoapps/student/views.py
+++ b/common/djangoapps/student/views.py
@@ -60,6 +60,10 @@ def index(request):
if settings.COURSEWARE_ENABLED and request.user.is_authenticated():
return redirect(reverse('dashboard'))
+ if settings.MITX_FEATURES.get('AUTH_USE_MIT_CERTIFICATES'):
+ from external_auth.views import edXauth_ssl_login
+ return edXauth_ssl_login(request)
+
return main_index()
def main_index(extra_context = {}):
diff --git a/lms/djangoapps/ssl_auth/__init__.py b/lms/djangoapps/ssl_auth/__init__.py
deleted file mode 100644
index e69de29bb2..0000000000
diff --git a/lms/djangoapps/ssl_auth/ssl_auth.py b/lms/djangoapps/ssl_auth/ssl_auth.py
deleted file mode 100755
index adbb2bf94d..0000000000
--- a/lms/djangoapps/ssl_auth/ssl_auth.py
+++ /dev/null
@@ -1,290 +0,0 @@
-"""
-User authentication backend for ssl (no pw required)
-"""
-
-from django.conf import settings
-from django.contrib import auth
-from django.contrib.auth.models import User, check_password
-from django.contrib.auth.backends import ModelBackend
-from django.contrib.auth.middleware import RemoteUserMiddleware
-from django.core.exceptions import ImproperlyConfigured
-import os
-import string
-import re
-from random import choice
-
-from student.models import UserProfile
-
-#-----------------------------------------------------------------------------
-
-
-def ssl_dn_extract_info(dn):
- '''
- Extract username, email address (may be anyuser@anydomain.com) and full name
- from the SSL DN string. Return (user,email,fullname) if successful, and None
- otherwise.
- '''
- ss = re.search('/emailAddress=(.*)@([^/]+)', dn)
- if ss:
- user = ss.group(1)
- email = "%s@%s" % (user, ss.group(2))
- else:
- return None
- ss = re.search('/CN=([^/]+)/', dn)
- if ss:
- fullname = ss.group(1)
- else:
- return None
- return (user, email, fullname)
-
-
-def check_nginx_proxy(request):
- '''
- Check for keys in the HTTP header (META) to se if we are behind an ngix reverse proxy.
- If so, get user info from the SSL DN string and return that, as (user,email,fullname)
- '''
- m = request.META
- if m.has_key('HTTP_X_REAL_IP'): # we're behind a nginx reverse proxy, which has already done ssl auth
- if not m.has_key('HTTP_SSL_CLIENT_S_DN'):
- return None
- dn = m['HTTP_SSL_CLIENT_S_DN']
- return ssl_dn_extract_info(dn)
- return None
-
-#-----------------------------------------------------------------------------
-
-
-def get_ssl_username(request):
- x = check_nginx_proxy(request)
- if x:
- return x[0]
- env = request._req.subprocess_env
- if env.has_key('SSL_CLIENT_S_DN_Email'):
- email = env['SSL_CLIENT_S_DN_Email']
- user = email[:email.index('@')]
- return user
- return None
-
-#-----------------------------------------------------------------------------
-
-
-class NginxProxyHeaderMiddleware(RemoteUserMiddleware):
- '''
- Django "middleware" function for extracting user information from HTTP request.
-
- '''
- # this field is generated by nginx's reverse proxy
- header = 'HTTP_SSL_CLIENT_S_DN' # specify the request.META field to use
-
- def process_request(self, request):
- # AuthenticationMiddleware is required so that request.user exists.
- if not hasattr(request, 'user'):
- raise ImproperlyConfigured(
- "The Django remote user auth middleware requires the"
- " authentication middleware to be installed. Edit your"
- " MIDDLEWARE_CLASSES setting to insert"
- " 'django.contrib.auth.middleware.AuthenticationMiddleware'"
- " before the RemoteUserMiddleware class.")
-
- #raise ImproperlyConfigured('[ProxyHeaderMiddleware] request.META=%s' % repr(request.META))
-
- try:
- username = request.META[self.header] # try the nginx META key first
- except KeyError:
- try:
- env = request._req.subprocess_env # else try the direct apache2 SSL key
- if env.has_key('SSL_CLIENT_S_DN'):
- username = env['SSL_CLIENT_S_DN']
- else:
- raise ImproperlyConfigured('no ssl key, env=%s' % repr(env))
- username = ''
- except:
- # If specified header doesn't exist then return (leaving
- # request.user set to AnonymousUser by the
- # AuthenticationMiddleware).
- return
- # If the user is already authenticated and that user is the user we are
- # getting passed in the headers, then the correct user is already
- # persisted in the session and we don't need to continue.
-
- #raise ImproperlyConfigured('[ProxyHeaderMiddleware] username=%s' % username)
-
- if request.user.is_authenticated():
- if request.user.username == self.clean_username(username, request):
- #raise ImproperlyConfigured('%s already authenticated (%s)' % (username,request.user.username))
- return
- # We are seeing this user for the first time in this session, attempt
- # to authenticate the user.
- #raise ImproperlyConfigured('calling auth.authenticate, remote_user=%s' % username)
- user = auth.authenticate(remote_user=username)
- if user:
- # User is valid. Set request.user and persist user in the session
- # by logging the user in.
- request.user = user
- if settings.DEBUG: print "[ssl_auth.ssl_auth.NginxProxyHeaderMiddleware] logging in user=%s" % user
- auth.login(request, user)
-
- def clean_username(self, username, request):
- '''
- username is the SSL DN string - extract the actual username from it and return
- '''
- info = ssl_dn_extract_info(username)
- if not info:
- return None
- (username, email, fullname) = info
- return username
-
-#-----------------------------------------------------------------------------
-
-
-class SSLLoginBackend(ModelBackend):
- '''
- Django authentication back-end which auto-logs-in a user based on having
- already authenticated with an MIT certificate (SSL).
- '''
- def authenticate(self, username=None, password=None, remote_user=None):
-
- # remote_user is from the SSL_DN string. It will be non-empty only when
- # the user has already passed the server authentication, which means
- # matching with the certificate authority.
- if not remote_user:
- # no remote_user, so check username (but don't auto-create user)
- if not username:
- return None
- return None # pass on to another authenticator backend
- #raise ImproperlyConfigured("in SSLLoginBackend, username=%s, remote_user=%s" % (username,remote_user))
- try:
- user = User.objects.get(username=username) # if user already exists don't create it
- return user
- except User.DoesNotExist:
- return None
- return None
-
- #raise ImproperlyConfigured("in SSLLoginBackend, username=%s, remote_user=%s" % (username,remote_user))
- #if not os.environ.has_key('HTTPS'):
- # return None
- #if not os.environ.get('HTTPS')=='on': # only use this back-end if HTTPS on
- # return None
-
- def GenPasswd(length=8, chars=string.letters + string.digits):
- return ''.join([choice(chars) for i in range(length)])
-
- # convert remote_user to user, email, fullname
- info = ssl_dn_extract_info(remote_user)
- #raise ImproperlyConfigured("[SSLLoginBackend] looking up %s" % repr(info))
- if not info:
- #raise ImproperlyConfigured("[SSLLoginBackend] remote_user=%s, info=%s" % (remote_user,info))
- return None
- (username, email, fullname) = info
-
- try:
- user = User.objects.get(username=username) # if user already exists don't create it
- except User.DoesNotExist:
- if not settings.DEBUG:
- raise "User does not exist. Not creating user; potential schema consistency issues"
- #raise ImproperlyConfigured("[SSLLoginBackend] creating %s" % repr(info))
- user = User(username=username, password=GenPasswd()) # create new User
- user.is_staff = False
- user.is_superuser = False
- # get first, last name from fullname
- name = fullname
- if not name.count(' '):
- user.first_name = " "
- user.last_name = name
- mn = ''
- else:
- user.first_name = name[:name.find(' ')]
- ml = name[name.find(' '):].strip()
- if ml.count(' '):
- user.last_name = ml[ml.rfind(' '):]
- mn = ml[:ml.rfind(' ')]
- else:
- user.last_name = ml
- mn = ''
- # set email
- user.email = email
- # cleanup last name
- user.last_name = user.last_name.strip()
- # save
- user.save()
-
- # auto-create user profile
- up = UserProfile(user=user)
- up.name = fullname
- up.save()
-
- #tui = user.get_profile()
- #tui.middle_name = mn
- #tui.role = 'Misc'
- #tui.section = None # no section assigned at first
- #tui.save()
- # return None
- return user
-
- def get_user(self, user_id):
- #if not os.environ.has_key('HTTPS'):
- # return None
- #if not os.environ.get('HTTPS')=='on': # only use this back-end if HTTPS on
- # return None
- try:
- return User.objects.get(pk=user_id)
- except User.DoesNotExist:
- return None
-
-#-----------------------------------------------------------------------------
-# OLD!
-
-
-class AutoLoginBackend:
- def authenticate(self, username=None, password=None):
- raise ImproperlyConfigured("in AutoLoginBackend, username=%s" % username)
- if not os.environ.has_key('HTTPS'):
- return None
- if not os.environ.get('HTTPS') == 'on':# only use this back-end if HTTPS on
- return None
-
- def GenPasswd(length=8, chars=string.letters + string.digits):
- return ''.join([choice(chars) for i in range(length)])
-
- try:
- user = User.objects.get(username=username)
- except User.DoesNotExist:
- user = User(username=username, password=GenPasswd())
- user.is_staff = False
- user.is_superuser = False
- # get first, last name
- name = os.environ.get('SSL_CLIENT_S_DN_CN').strip()
- if not name.count(' '):
- user.first_name = " "
- user.last_name = name
- mn = ''
- else:
- user.first_name = name[:name.find(' ')]
- ml = name[name.find(' '):].strip()
- if ml.count(' '):
- user.last_name = ml[ml.rfind(' '):]
- mn = ml[:ml.rfind(' ')]
- else:
- user.last_name = ml
- mn = ''
- # get email
- user.email = os.environ.get('SSL_CLIENT_S_DN_Email')
- # save
- user.save()
- tui = user.get_profile()
- tui.middle_name = mn
- tui.role = 'Misc'
- tui.section = None# no section assigned at first
- tui.save()
- # return None
- return user
-
- def get_user(self, user_id):
- if not os.environ.has_key('HTTPS'):
- return None
- if not os.environ.get('HTTPS') == 'on':# only use this back-end if HTTPS on
- return None
- try:
- return User.objects.get(pk=user_id)
- except User.DoesNotExist:
- return None
diff --git a/lms/envs/dev.py b/lms/envs/dev.py
index f9b7ba10a0..83bc596f1e 100644
--- a/lms/envs/dev.py
+++ b/lms/envs/dev.py
@@ -67,6 +67,9 @@ OPENID_UPDATE_DETAILS_FROM_SREG = True
OPENID_SSO_SERVER_URL = 'https://www.google.com/accounts/o8/id' # TODO: accept more endpoints
OPENID_USE_AS_ADMIN_LOGIN = False
+################################ MIT Certificates SSL Auth #################################
+MITX_FEATURES['AUTH_USE_MIT_CERTIFICATES'] = True
+
################################ DEBUG TOOLBAR #################################
INSTALLED_APPS += ('debug_toolbar',)
MIDDLEWARE_CLASSES += ('debug_toolbar.middleware.DebugToolbarMiddleware',)
From 4a0d0a08db20fe64fa0dceb08d157cce1cfaa026 Mon Sep 17 00:00:00 2001
From: ichuang
Date: Wed, 1 Aug 2012 23:37:35 -0400
Subject: [PATCH 071/124] minor change so that SSL code doesn't interfere with
non-nginx instances
---
common/djangoapps/external_auth/views.py | 5 ++++-
1 file changed, 4 insertions(+), 1 deletion(-)
diff --git a/common/djangoapps/external_auth/views.py b/common/djangoapps/external_auth/views.py
index 5004d614d5..41062735d4 100644
--- a/common/djangoapps/external_auth/views.py
+++ b/common/djangoapps/external_auth/views.py
@@ -194,7 +194,10 @@ def edXauth_ssl_login(request):
if not cert:
cert = request.META.get('HTTP_'+certkey,'')
if not cert:
- cert = request._req.subprocess_env.get(certkey,'') # try the direct apache2 SSL key
+ try:
+ cert = request._req.subprocess_env.get(certkey,'') # try the direct apache2 SSL key
+ except Exception as err:
+ pass
if not cert:
# no certificate information - go onward to main index
return student_views.main_index()
From 727e51411f7e096c92745c3deb630bd77b2f119c Mon Sep 17 00:00:00 2001
From: ichuang
Date: Thu, 2 Aug 2012 08:59:02 -0400
Subject: [PATCH 072/124] small change so that ssl authenticated user can
logout to see main screen
---
common/djangoapps/external_auth/views.py | 11 ++++++++---
1 file changed, 8 insertions(+), 3 deletions(-)
diff --git a/common/djangoapps/external_auth/views.py b/common/djangoapps/external_auth/views.py
index 41062735d4..55ff4b4194 100644
--- a/common/djangoapps/external_auth/views.py
+++ b/common/djangoapps/external_auth/views.py
@@ -80,7 +80,8 @@ def edXauth_openid_login_complete(request, redirect_field_name=REDIRECT_FIELD_N
#-----------------------------------------------------------------------------
# generic external auth login or signup
-def edXauth_external_login_or_signup(request, external_id, external_domain, credentials, email, fullname):
+def edXauth_external_login_or_signup(request, external_id, external_domain, credentials, email, fullname,
+ retfun=None):
# see if we have a map from this external_id to an edX username
try:
eamap = ExternalAuthMap.objects.get(external_id=external_id)
@@ -118,7 +119,10 @@ def edXauth_external_login_or_signup(request, external_id, external_domain, cred
request.session.set_expiry(0)
student_views.try_change_enrollment(request)
log.info("Login success - {0} ({1})".format(user.username, user.email))
- return redirect('/')
+ if retfun is None:
+ return redirect('/')
+ return retfun()
+
#-----------------------------------------------------------------------------
# generic external auth signup
@@ -209,4 +213,5 @@ def edXauth_ssl_login(request):
external_domain="ssl:MIT",
credentials=cert,
email=email,
- fullname=fullname)
+ fullname=fullname,
+ retfun = student_views.main_index)
From 052ae881110f7e127b3566f6b2c7c6e41e8df177 Mon Sep 17 00:00:00 2001
From: Calen Pennington
Date: Thu, 2 Aug 2012 09:19:25 -0400
Subject: [PATCH 073/124] Only import from github when changes are made, rather
than exporting. This prevents bugs in the cms from preventing changes via git
---
cms/djangoapps/github_sync/views.py | 4 ++--
1 file changed, 2 insertions(+), 2 deletions(-)
diff --git a/cms/djangoapps/github_sync/views.py b/cms/djangoapps/github_sync/views.py
index 941d50f986..c3b5172b29 100644
--- a/cms/djangoapps/github_sync/views.py
+++ b/cms/djangoapps/github_sync/views.py
@@ -5,7 +5,7 @@ from django.http import HttpResponse
from django.conf import settings
from django_future.csrf import csrf_exempt
-from . import sync_with_github, load_repo_settings
+from . import import_from_github, load_repo_settings
log = logging.getLogger()
@@ -46,6 +46,6 @@ def github_post_receive(request):
log.info('Ignoring changes to non-tracked branch %s in repo %s' % (branch_name, repo_name))
return HttpResponse('Ignoring non-tracked branch')
- sync_with_github(repo)
+ import_from_github(repo)
return HttpResponse('Push received')
From 23c3c5a6529db587e505aed01f908ea6684e3b8c Mon Sep 17 00:00:00 2001
From: ichuang
Date: Thu, 2 Aug 2012 09:37:24 -0400
Subject: [PATCH 074/124] print -> log.debug, rename function from camel case
---
common/djangoapps/external_auth/views.py | 8 ++++----
1 file changed, 4 insertions(+), 4 deletions(-)
diff --git a/common/djangoapps/external_auth/views.py b/common/djangoapps/external_auth/views.py
index 55ff4b4194..fdffaf0c1d 100644
--- a/common/djangoapps/external_auth/views.py
+++ b/common/djangoapps/external_auth/views.py
@@ -35,16 +35,16 @@ log = logging.getLogger("mitx.external_auth")
@csrf_exempt
def default_render_failure(request, message, status=403, template_name='extauth_failure.html', exception=None):
"""Render an Openid error page to the user."""
-
message = "In openid_failure " + message
- print "in openid_failure: "
+ log.debug(message)
data = render_to_string( template_name, dict(message=message, exception=exception))
return HttpResponse(data, status=status)
#-----------------------------------------------------------------------------
# Openid
-def GenPasswd(length=12, chars=string.letters + string.digits):
+def edXauth_generate_password(length=12, chars=string.letters + string.digits):
+ """Generate internal password for externally authenticated user"""
return ''.join([random.choice(chars) for i in range(length)])
@csrf_exempt
@@ -94,7 +94,7 @@ def edXauth_external_login_or_signup(request, external_id, external_domain, cred
)
eamap.external_email = email
eamap.external_name = fullname
- eamap.internal_password = GenPasswd()
+ eamap.internal_password = edXauth_generate_password()
log.debug('created eamap=%s' % eamap)
eamap.save()
From b2e9d980ff4840539fd593f43c175f1040221d3b Mon Sep 17 00:00:00 2001
From: ichuang
Date: Thu, 2 Aug 2012 09:42:26 -0400
Subject: [PATCH 075/124] don't overwrite oid_backend
---
common/djangoapps/external_auth/views.py | 2 +-
1 file changed, 1 insertion(+), 1 deletion(-)
diff --git a/common/djangoapps/external_auth/views.py b/common/djangoapps/external_auth/views.py
index fdffaf0c1d..827f2bc4c5 100644
--- a/common/djangoapps/external_auth/views.py
+++ b/common/djangoapps/external_auth/views.py
@@ -63,7 +63,7 @@ def edXauth_openid_login_complete(request, redirect_field_name=REDIRECT_FIELD_N
if openid_response.status == SUCCESS:
external_id = openid_response.identity_url
oid_backend = openid_auth.OpenIDBackend()
- details = oid_backened = oid_backend._extract_user_details(openid_response)
+ details = oid_backend._extract_user_details(openid_response)
log.debug('openid success, details=%s' % details)
From fc0f938eae4280c714ad870459e7a05410131840 Mon Sep 17 00:00:00 2001
From: Victor Shnayder
Date: Wed, 1 Aug 2012 20:44:27 -0400
Subject: [PATCH 076/124] Responding to comments on pull #326
* cleaned up error module:
- only one template
- save error message in xml and reload
* better display of problem definition and metadata to staff
* save error messages as string, not exception objects.
---
common/djangoapps/xmodule_modifiers.py | 4 +-
common/lib/xmodule/tests/test_import.py | 1 +
common/lib/xmodule/xmodule/error_module.py | 49 +++++++++++++---------
common/lib/xmodule/xmodule/errortracker.py | 8 +++-
common/lib/xmodule/xmodule/x_module.py | 14 ++++---
lms/templates/module-error-staff.html | 11 -----
lms/templates/module-error.html | 9 ++++
7 files changed, 57 insertions(+), 39 deletions(-)
delete mode 100644 lms/templates/module-error-staff.html
diff --git a/common/djangoapps/xmodule_modifiers.py b/common/djangoapps/xmodule_modifiers.py
index b942d6726b..4d412000ec 100644
--- a/common/djangoapps/xmodule_modifiers.py
+++ b/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),
diff --git a/common/lib/xmodule/tests/test_import.py b/common/lib/xmodule/tests/test_import.py
index 6a407fe189..0d3e2260fb 100644
--- a/common/lib/xmodule/tests/test_import.py
+++ b/common/lib/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 = ''''''
system = self.get_system()
descriptor = XModuleDescriptor.load_from_xml(bad_xml, system, 'org', 'course',
diff --git a/common/lib/xmodule/xmodule/error_module.py b/common/lib/xmodule/xmodule/error_module.py
index 62b3b82a39..ecc90873b9 100644
--- a/common/lib/xmodule/xmodule/error_module.py
+++ b/common/lib/xmodule/xmodule/error_module.py
@@ -1,11 +1,14 @@
+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:
- # Save the error to display later--overrides other problems
- definition['error'] = 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'
- definition['data'] = xml_data
+ 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']
@@ -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)
diff --git a/common/lib/xmodule/xmodule/errortracker.py b/common/lib/xmodule/xmodule/errortracker.py
index b8d42a6983..8ac2903149 100644
--- a/common/lib/xmodule/xmodule/errortracker.py
+++ b/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))
diff --git a/common/lib/xmodule/xmodule/x_module.py b/common/lib/xmodule/xmodule/x_module.py
index 360f1b07d0..ac6b5db5a4 100644
--- a/common/lib/xmodule/xmodule/x_module.py
+++ b/common/lib/xmodule/xmodule/x_module.py
@@ -1,12 +1,14 @@
-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
diff --git a/lms/templates/module-error-staff.html b/lms/templates/module-error-staff.html
deleted file mode 100644
index 955f413db3..0000000000
--- a/lms/templates/module-error-staff.html
+++ /dev/null
@@ -1,11 +0,0 @@
-
-
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.
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
+
From f2a9110bdaa804bf20a91d9020113f2657d05c76 Mon Sep 17 00:00:00 2001
From: ichuang
Date: Thu, 2 Aug 2012 09:56:33 -0400
Subject: [PATCH 077/124] change model to have external_id and external_domain
be unique_together
---
common/djangoapps/external_auth/models.py | 4 +++-
1 file changed, 3 insertions(+), 1 deletion(-)
diff --git a/common/djangoapps/external_auth/models.py b/common/djangoapps/external_auth/models.py
index b8b167d25d..e43b306bbb 100644
--- a/common/djangoapps/external_auth/models.py
+++ b/common/djangoapps/external_auth/models.py
@@ -13,7 +13,9 @@ from django.db import models
from django.contrib.auth.models import User
class ExternalAuthMap(models.Model):
- external_id = models.CharField(max_length=255, db_index=True, unique=True)
+ class Meta:
+ unique_together = (('external_id', 'external_domain'), )
+ external_id = models.CharField(max_length=255, db_index=True)
external_domain = models.CharField(max_length=255, db_index=True)
external_credentials = models.TextField(blank=True) # JSON dictionary
external_email = models.CharField(max_length=255, db_index=True)
From 613c53a7109b9162acc439e202b3430f099ee35f Mon Sep 17 00:00:00 2001
From: ichuang
Date: Thu, 2 Aug 2012 10:05:26 -0400
Subject: [PATCH 078/124] slight cleanup, no need to import all of
django_openid_auth
---
common/djangoapps/external_auth/views.py | 4 ++--
1 file changed, 2 insertions(+), 2 deletions(-)
diff --git a/common/djangoapps/external_auth/views.py b/common/djangoapps/external_auth/views.py
index 827f2bc4c5..740b4ed1ac 100644
--- a/common/djangoapps/external_auth/views.py
+++ b/common/djangoapps/external_auth/views.py
@@ -26,7 +26,7 @@ from util.cache import cache_if_anonymous
from django_openid_auth import auth as openid_auth
from openid.consumer.consumer import (Consumer, SUCCESS, CANCEL, FAILURE)
-import django_openid_auth
+import django_openid_auth.views as openid_views
import student.views as student_views
@@ -56,7 +56,7 @@ def edXauth_openid_login_complete(request, redirect_field_name=REDIRECT_FIELD_N
getattr(settings, 'OPENID_RENDER_FAILURE', None) or \
default_render_failure
- openid_response = django_openid_auth.views.parse_openid_response(request)
+ openid_response = openid_views.parse_openid_response(request)
if not openid_response:
return render_failure(request, 'This is an OpenID relying party endpoint.')
From 41eca8a0a54a974b6a2b231e786ffd3573e4cd81 Mon Sep 17 00:00:00 2001
From: Victor Shnayder
Date: Thu, 2 Aug 2012 10:13:58 -0400
Subject: [PATCH 079/124] Move files into xmodule module per comment on #326
* move html_checker.py
* move stringify.py
* move tests into module
* remove duplicate progress.py
* fix module imports
---
common/lib/xmodule/progress.py | 157 ------------------
common/lib/xmodule/tests/test_stringify.py | 9 -
.../lib/xmodule/{ => xmodule}/html_checker.py | 0
common/lib/xmodule/xmodule/html_module.py | 10 +-
common/lib/xmodule/{ => xmodule}/stringify.py | 0
.../xmodule/{ => xmodule}/tests/__init__.py | 0
.../{ => xmodule}/tests/test_export.py | 0
.../test_files/choiceresponse_checkbox.xml | 0
.../tests/test_files/choiceresponse_radio.xml | 0
.../tests/test_files/coderesponse.xml | 0
.../test_files/formularesponse_with_hint.xml | 0
.../tests/test_files/imageresponse.xml | 0
.../tests/test_files/multi_bare.xml | 0
.../tests/test_files/multichoice.xml | 0
.../tests/test_files/optionresponse.xml | 0
.../test_files/stringresponse_with_hint.xml | 0
.../tests/test_files/symbolicresponse.xml | 0
.../tests/test_files/truefalse.xml | 0
.../{ => xmodule}/tests/test_import.py | 0
.../xmodule/xmodule/tests/test_stringify.py | 10 ++
20 files changed, 15 insertions(+), 171 deletions(-)
delete mode 100644 common/lib/xmodule/progress.py
delete mode 100644 common/lib/xmodule/tests/test_stringify.py
rename common/lib/xmodule/{ => xmodule}/html_checker.py (100%)
rename common/lib/xmodule/{ => xmodule}/stringify.py (100%)
rename common/lib/xmodule/{ => xmodule}/tests/__init__.py (100%)
rename common/lib/xmodule/{ => xmodule}/tests/test_export.py (100%)
rename common/lib/xmodule/{ => xmodule}/tests/test_files/choiceresponse_checkbox.xml (100%)
rename common/lib/xmodule/{ => xmodule}/tests/test_files/choiceresponse_radio.xml (100%)
rename common/lib/xmodule/{ => xmodule}/tests/test_files/coderesponse.xml (100%)
rename common/lib/xmodule/{ => xmodule}/tests/test_files/formularesponse_with_hint.xml (100%)
rename common/lib/xmodule/{ => xmodule}/tests/test_files/imageresponse.xml (100%)
rename common/lib/xmodule/{ => xmodule}/tests/test_files/multi_bare.xml (100%)
rename common/lib/xmodule/{ => xmodule}/tests/test_files/multichoice.xml (100%)
rename common/lib/xmodule/{ => xmodule}/tests/test_files/optionresponse.xml (100%)
rename common/lib/xmodule/{ => xmodule}/tests/test_files/stringresponse_with_hint.xml (100%)
rename common/lib/xmodule/{ => xmodule}/tests/test_files/symbolicresponse.xml (100%)
rename common/lib/xmodule/{ => xmodule}/tests/test_files/truefalse.xml (100%)
rename common/lib/xmodule/{ => xmodule}/tests/test_import.py (100%)
create mode 100644 common/lib/xmodule/xmodule/tests/test_stringify.py
diff --git a/common/lib/xmodule/progress.py b/common/lib/xmodule/progress.py
deleted file mode 100644
index 70c8ec9da1..0000000000
--- a/common/lib/xmodule/progress.py
+++ /dev/null
@@ -1,157 +0,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)
diff --git a/common/lib/xmodule/tests/test_stringify.py b/common/lib/xmodule/tests/test_stringify.py
deleted file mode 100644
index 62d7683886..0000000000
--- a/common/lib/xmodule/tests/test_stringify.py
+++ /dev/null
@@ -1,9 +0,0 @@
-from nose.tools import assert_equals
-from lxml import etree
-from stringify import stringify_children
-
-def test_stringify():
- html = '''Hi
there Bruce!
'''
- xml = etree.fromstring(html)
- out = stringify_children(xml)
- assert_equals(out, '''Hi
there Bruce!
''')
diff --git a/common/lib/xmodule/html_checker.py b/common/lib/xmodule/xmodule/html_checker.py
similarity index 100%
rename from common/lib/xmodule/html_checker.py
rename to common/lib/xmodule/xmodule/html_checker.py
diff --git a/common/lib/xmodule/xmodule/html_module.py b/common/lib/xmodule/xmodule/html_module.py
index b2a5df9803..7c3456e5ad 100644
--- a/common/lib/xmodule/xmodule/html_module.py
+++ b/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")
diff --git a/common/lib/xmodule/stringify.py b/common/lib/xmodule/xmodule/stringify.py
similarity index 100%
rename from common/lib/xmodule/stringify.py
rename to common/lib/xmodule/xmodule/stringify.py
diff --git a/common/lib/xmodule/tests/__init__.py b/common/lib/xmodule/xmodule/tests/__init__.py
similarity index 100%
rename from common/lib/xmodule/tests/__init__.py
rename to common/lib/xmodule/xmodule/tests/__init__.py
diff --git a/common/lib/xmodule/tests/test_export.py b/common/lib/xmodule/xmodule/tests/test_export.py
similarity index 100%
rename from common/lib/xmodule/tests/test_export.py
rename to common/lib/xmodule/xmodule/tests/test_export.py
diff --git a/common/lib/xmodule/tests/test_files/choiceresponse_checkbox.xml b/common/lib/xmodule/xmodule/tests/test_files/choiceresponse_checkbox.xml
similarity index 100%
rename from common/lib/xmodule/tests/test_files/choiceresponse_checkbox.xml
rename to common/lib/xmodule/xmodule/tests/test_files/choiceresponse_checkbox.xml
diff --git a/common/lib/xmodule/tests/test_files/choiceresponse_radio.xml b/common/lib/xmodule/xmodule/tests/test_files/choiceresponse_radio.xml
similarity index 100%
rename from common/lib/xmodule/tests/test_files/choiceresponse_radio.xml
rename to common/lib/xmodule/xmodule/tests/test_files/choiceresponse_radio.xml
diff --git a/common/lib/xmodule/tests/test_files/coderesponse.xml b/common/lib/xmodule/xmodule/tests/test_files/coderesponse.xml
similarity index 100%
rename from common/lib/xmodule/tests/test_files/coderesponse.xml
rename to common/lib/xmodule/xmodule/tests/test_files/coderesponse.xml
diff --git a/common/lib/xmodule/tests/test_files/formularesponse_with_hint.xml b/common/lib/xmodule/xmodule/tests/test_files/formularesponse_with_hint.xml
similarity index 100%
rename from common/lib/xmodule/tests/test_files/formularesponse_with_hint.xml
rename to common/lib/xmodule/xmodule/tests/test_files/formularesponse_with_hint.xml
diff --git a/common/lib/xmodule/tests/test_files/imageresponse.xml b/common/lib/xmodule/xmodule/tests/test_files/imageresponse.xml
similarity index 100%
rename from common/lib/xmodule/tests/test_files/imageresponse.xml
rename to common/lib/xmodule/xmodule/tests/test_files/imageresponse.xml
diff --git a/common/lib/xmodule/tests/test_files/multi_bare.xml b/common/lib/xmodule/xmodule/tests/test_files/multi_bare.xml
similarity index 100%
rename from common/lib/xmodule/tests/test_files/multi_bare.xml
rename to common/lib/xmodule/xmodule/tests/test_files/multi_bare.xml
diff --git a/common/lib/xmodule/tests/test_files/multichoice.xml b/common/lib/xmodule/xmodule/tests/test_files/multichoice.xml
similarity index 100%
rename from common/lib/xmodule/tests/test_files/multichoice.xml
rename to common/lib/xmodule/xmodule/tests/test_files/multichoice.xml
diff --git a/common/lib/xmodule/tests/test_files/optionresponse.xml b/common/lib/xmodule/xmodule/tests/test_files/optionresponse.xml
similarity index 100%
rename from common/lib/xmodule/tests/test_files/optionresponse.xml
rename to common/lib/xmodule/xmodule/tests/test_files/optionresponse.xml
diff --git a/common/lib/xmodule/tests/test_files/stringresponse_with_hint.xml b/common/lib/xmodule/xmodule/tests/test_files/stringresponse_with_hint.xml
similarity index 100%
rename from common/lib/xmodule/tests/test_files/stringresponse_with_hint.xml
rename to common/lib/xmodule/xmodule/tests/test_files/stringresponse_with_hint.xml
diff --git a/common/lib/xmodule/tests/test_files/symbolicresponse.xml b/common/lib/xmodule/xmodule/tests/test_files/symbolicresponse.xml
similarity index 100%
rename from common/lib/xmodule/tests/test_files/symbolicresponse.xml
rename to common/lib/xmodule/xmodule/tests/test_files/symbolicresponse.xml
diff --git a/common/lib/xmodule/tests/test_files/truefalse.xml b/common/lib/xmodule/xmodule/tests/test_files/truefalse.xml
similarity index 100%
rename from common/lib/xmodule/tests/test_files/truefalse.xml
rename to common/lib/xmodule/xmodule/tests/test_files/truefalse.xml
diff --git a/common/lib/xmodule/tests/test_import.py b/common/lib/xmodule/xmodule/tests/test_import.py
similarity index 100%
rename from common/lib/xmodule/tests/test_import.py
rename to common/lib/xmodule/xmodule/tests/test_import.py
diff --git a/common/lib/xmodule/xmodule/tests/test_stringify.py b/common/lib/xmodule/xmodule/tests/test_stringify.py
new file mode 100644
index 0000000000..1c6ee855f3
--- /dev/null
+++ b/common/lib/xmodule/xmodule/tests/test_stringify.py
@@ -0,0 +1,10 @@
+from nose.tools import assert_equals
+from lxml import etree
+from xmodule.stringify import stringify_children
+
+def test_stringify():
+ text = 'Hi
there Bruce!
'
+ html = '''{0}'''.format(text)
+ xml = etree.fromstring(html)
+ out = stringify_children(xml)
+ assert_equals(out, text)
From 31b8270cfb1442be97ada91b157f67690125a2a2 Mon Sep 17 00:00:00 2001
From: Victor Shnayder
Date: Thu, 2 Aug 2012 10:14:35 -0400
Subject: [PATCH 080/124] remove too-clever lazy dictionary
---
common/lib/xmodule/lazy_dict.py | 58 ---------------------------------
1 file changed, 58 deletions(-)
delete mode 100644 common/lib/xmodule/lazy_dict.py
diff --git a/common/lib/xmodule/lazy_dict.py b/common/lib/xmodule/lazy_dict.py
deleted file mode 100644
index fa614843ec..0000000000
--- a/common/lib/xmodule/lazy_dict.py
+++ /dev/null
@@ -1,58 +0,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
-
From fd796478d83fbbd9571710edc3b21eb7b4b96d5a Mon Sep 17 00:00:00 2001
From: Victor Shnayder
Date: Thu, 2 Aug 2012 11:33:04 -0400
Subject: [PATCH 081/124] add a handy supertrace script
---
common/lib/supertrace.py | 52 ++++++++++++++++++++++++++++++++++++++++
1 file changed, 52 insertions(+)
create mode 100644 common/lib/supertrace.py
diff --git a/common/lib/supertrace.py b/common/lib/supertrace.py
new file mode 100644
index 0000000000..e17cd7a8ba
--- /dev/null
+++ b/common/lib/supertrace.py
@@ -0,0 +1,52 @@
+"""
+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 ""
+
+
From 64346d727b3c142ad74de87e2fa37c12667e236f Mon Sep 17 00:00:00 2001
From: Victor Shnayder
Date: Thu, 2 Aug 2012 11:33:53 -0400
Subject: [PATCH 082/124] todos and comments
---
common/lib/xmodule/xmodule/editing_module.py | 7 ++++++-
lms/djangoapps/courseware/views.py | 5 +++--
2 files changed, 9 insertions(+), 3 deletions(-)
diff --git a/common/lib/xmodule/xmodule/editing_module.py b/common/lib/xmodule/xmodule/editing_module.py
index 4188165a24..67a4d66dad 100644
--- a/common/lib/xmodule/xmodule/editing_module.py
+++ b/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,
}
diff --git a/lms/djangoapps/courseware/views.py b/lms/djangoapps/courseware/views.py
index a049638d1b..2b55b48caf 100644
--- a/lms/djangoapps/courseware/views.py
+++ b/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
From 3eff9ffecd2382ba0bad9b877376388daffa7a8e Mon Sep 17 00:00:00 2001
From: ichuang
Date: Thu, 2 Aug 2012 13:28:52 -0400
Subject: [PATCH 083/124] match external_domain as well when retrieving
ExternalAuthMap objects
---
common/djangoapps/external_auth/views.py | 4 +++-
1 file changed, 3 insertions(+), 1 deletion(-)
diff --git a/common/djangoapps/external_auth/views.py b/common/djangoapps/external_auth/views.py
index 740b4ed1ac..d00a0a7182 100644
--- a/common/djangoapps/external_auth/views.py
+++ b/common/djangoapps/external_auth/views.py
@@ -84,7 +84,9 @@ def edXauth_external_login_or_signup(request, external_id, external_domain, cred
retfun=None):
# see if we have a map from this external_id to an edX username
try:
- eamap = ExternalAuthMap.objects.get(external_id=external_id)
+ eamap = ExternalAuthMap.objects.get(external_id = external_id,
+ external_domain = external_domain,
+ )
log.debug('Found eamap=%s' % eamap)
except ExternalAuthMap.DoesNotExist:
# go render form for creating edX user
From a7103ff8932ddfdcd10e844aa71fd3901690de47 Mon Sep 17 00:00:00 2001
From: ichuang
Date: Thu, 2 Aug 2012 13:39:12 -0400
Subject: [PATCH 084/124] switch to PascalCase, remove unnecessary assignment
---
common/djangoapps/student/views.py | 8 ++++----
1 file changed, 4 insertions(+), 4 deletions(-)
diff --git a/common/djangoapps/student/views.py b/common/djangoapps/student/views.py
index 7937d67980..35ce225011 100644
--- a/common/djangoapps/student/views.py
+++ b/common/djangoapps/student/views.py
@@ -280,14 +280,14 @@ def create_account(request, post_override=None):
# if doing signup for an external authorization, then get email, password, name from the eamap
# don't use the ones from the form, since the user could have hacked those
- doExternalAuth = 'ExternalAuthMap' in request.session
- if doExternalAuth:
+ DoExternalAuth = 'ExternalAuthMap' in request.session
+ if DoExternalAuth:
eamap = request.session['ExternalAuthMap']
email = eamap.external_email
name = eamap.external_name
password = eamap.internal_password
post_vars = dict(post_vars.items())
- post_vars.update(dict(email=email, name=name, password=password, username=post_vars['username']))
+ post_vars.update(dict(email=email, name=name, password=password))
log.debug('extauth test: post_vars = %s' % post_vars)
# Confirm we have a properly formed request
@@ -411,7 +411,7 @@ def create_account(request, post_override=None):
try_change_enrollment(request)
- if doExternalAuth:
+ if DoExternalAuth:
eamap.user = login_user
eamap.dtsignup = datetime.datetime.now()
eamap.save()
From 26ae88faac8c620fa14d0cabfee7c03ac5938169 Mon Sep 17 00:00:00 2001
From: Victor Shnayder
Date: Thu, 2 Aug 2012 13:41:54 -0400
Subject: [PATCH 085/124] Factor out get_item_error() into a new
ModuleStoreBase class
* may be temporary if we move errors into the items themselves.
---
.../xmodule/xmodule/modulestore/__init__.py | 44 +++++++++++++++++--
.../lib/xmodule/xmodule/modulestore/mongo.py | 9 ++--
common/lib/xmodule/xmodule/modulestore/xml.py | 36 +++++----------
3 files changed, 59 insertions(+), 30 deletions(-)
diff --git a/common/lib/xmodule/xmodule/modulestore/__init__.py b/common/lib/xmodule/xmodule/modulestore/__init__.py
index 7241179d8e..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, InsufficientSpecificationError
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')
@@ -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
diff --git a/common/lib/xmodule/xmodule/modulestore/mongo.py b/common/lib/xmodule/xmodule/modulestore/mongo.py
index 76769b25b0..1cec6c7f87 100644
--- a/common/lib/xmodule/xmodule/modulestore/mongo.py
+++ b/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
diff --git a/common/lib/xmodule/xmodule/modulestore/xml.py b/common/lib/xmodule/xmodule/modulestore/xml.py
index d6540023e8..0567e4e7a7 100644
--- a/common/lib/xmodule/xmodule/modulestore/xml.py
+++ b/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
From 46bc7bc499fb63005d5225a465470c9d7481aa3d Mon Sep 17 00:00:00 2001
From: ichuang
Date: Thu, 2 Aug 2012 13:44:37 -0400
Subject: [PATCH 086/124] remove unnecessary hidden fields
---
lms/templates/signup_modal.html | 6 ------
1 file changed, 6 deletions(-)
diff --git a/lms/templates/signup_modal.html b/lms/templates/signup_modal.html
index 346027418d..1510eb407b 100644
--- a/lms/templates/signup_modal.html
+++ b/lms/templates/signup_modal.html
@@ -30,15 +30,9 @@
% else:
Welcome ${extauth_email}
-
-
Enter a public username:
-
-
-
-
% endif
From 10b2c212b6d9159acf430f8d65738471292eea5c Mon Sep 17 00:00:00 2001
From: ichuang
Date: Thu, 2 Aug 2012 13:52:25 -0400
Subject: [PATCH 087/124] fix javascript for signup modal .click() in
index.html for safari
---
lms/templates/index.html | 21 ++++++++++++++++++++-
1 file changed, 20 insertions(+), 1 deletion(-)
diff --git a/lms/templates/index.html b/lms/templates/index.html
index dcc18d4de1..4255251604 100644
--- a/lms/templates/index.html
+++ b/lms/templates/index.html
@@ -148,7 +148,26 @@
% if show_signup_immediately is not UNDEFINED:
From dff380e8076a49a82d10bda267d89e9d7259133d Mon Sep 17 00:00:00 2001
From: ichuang
Date: Thu, 2 Aug 2012 14:19:56 -0400
Subject: [PATCH 088/124] cleanup lms urls (remove cruft from debugging openid)
---
lms/urls.py | 4 ----
1 file changed, 4 deletions(-)
diff --git a/lms/urls.py b/lms/urls.py
index 8c36857ee3..35a71920a5 100644
--- a/lms/urls.py
+++ b/lms/urls.py
@@ -166,10 +166,6 @@ if settings.MITX_FEATURES.get('AUTH_USE_OPENID'):
url(r'^openid/complete/$', 'external_auth.views.edXauth_openid_login_complete', name='openid-complete'),
url(r'^openid/logo.gif$', 'django_openid_auth.views.logo', name='openid-logo'),
)
- urlpatterns += (
- url(r'^extauth/$', 'external_auth.views.edXauth_signup', name='extauth-signup'),
- )
- # urlpatterns += (url(r'^openid/', include('django_openid_auth.urls')),)
urlpatterns = patterns(*urlpatterns)
From 3b6f33f5a79c76d4bed9cad90b663bcf8deb7342 Mon Sep 17 00:00:00 2001
From: Victor Shnayder
Date: Thu, 2 Aug 2012 14:26:24 -0400
Subject: [PATCH 089/124] Fix github_sync tests
* CMS no longer syncs, it just imports when it gets a commit note
from github
---
.../github_sync/tests/test_views.py | 24 +++++++++----------
1 file changed, 12 insertions(+), 12 deletions(-)
diff --git a/cms/djangoapps/github_sync/tests/test_views.py b/cms/djangoapps/github_sync/tests/test_views.py
index 212d707340..37030d6a1b 100644
--- a/cms/djangoapps/github_sync/tests/test_views.py
+++ b/cms/djangoapps/github_sync/tests/test_views.py
@@ -11,33 +11,33 @@ class PostReceiveTestCase(TestCase):
def setUp(self):
self.client = Client()
- @patch('github_sync.views.sync_with_github')
- def test_non_branch(self, sync_with_github):
+ @patch('github_sync.views.import_from_github')
+ def test_non_branch(self, import_from_github):
self.client.post('/github_service_hook', {'payload': json.dumps({
'ref': 'refs/tags/foo'})
})
- self.assertFalse(sync_with_github.called)
+ self.assertFalse(import_from_github.called)
- @patch('github_sync.views.sync_with_github')
- def test_non_watched_repo(self, sync_with_github):
+ @patch('github_sync.views.import_from_github')
+ def test_non_watched_repo(self, import_from_github):
self.client.post('/github_service_hook', {'payload': json.dumps({
'ref': 'refs/heads/branch',
'repository': {'name': 'bad_repo'}})
})
- self.assertFalse(sync_with_github.called)
+ self.assertFalse(import_from_github.called)
- @patch('github_sync.views.sync_with_github')
- def test_non_tracked_branch(self, sync_with_github):
+ @patch('github_sync.views.import_from_github')
+ def test_non_tracked_branch(self, import_from_github):
self.client.post('/github_service_hook', {'payload': json.dumps({
'ref': 'refs/heads/non_branch',
'repository': {'name': 'repo'}})
})
- self.assertFalse(sync_with_github.called)
+ self.assertFalse(import_from_github.called)
- @patch('github_sync.views.sync_with_github')
- def test_tracked_branch(self, sync_with_github):
+ @patch('github_sync.views.import_from_github')
+ def test_tracked_branch(self, import_from_github):
self.client.post('/github_service_hook', {'payload': json.dumps({
'ref': 'refs/heads/branch',
'repository': {'name': 'repo'}})
})
- sync_with_github.assert_called_with(load_repo_settings('repo'))
+ import_from_github.assert_called_with(load_repo_settings('repo'))
From 987b9c11a94372f6d4c888755a1ce5ff5a036e30 Mon Sep 17 00:00:00 2001
From: Victor Shnayder
Date: Thu, 2 Aug 2012 14:05:42 -0400
Subject: [PATCH 090/124] Use url_name for chapters and sections in lms views
* got rid of the hackish conversions between ' ' and '_'
* use url_name and display_name where appropriate
* update templates to match.
---
common/lib/xmodule/xmodule/x_module.py | 2 ++
lms/djangoapps/courseware/grades.py | 33 ++++++++++++++--------
lms/djangoapps/courseware/module_render.py | 28 ++++++++++--------
lms/djangoapps/courseware/views.py | 29 ++++---------------
lms/templates/accordion.html | 6 ++--
lms/templates/profile.html | 28 +++++++++---------
6 files changed, 62 insertions(+), 64 deletions(-)
diff --git a/common/lib/xmodule/xmodule/x_module.py b/common/lib/xmodule/xmodule/x_module.py
index ac6b5db5a4..1d16849d67 100644
--- a/common/lib/xmodule/xmodule/x_module.py
+++ b/common/lib/xmodule/xmodule/x_module.py
@@ -339,6 +339,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
diff --git a/lms/djangoapps/courseware/grades.py b/lms/djangoapps/courseware/grades.py
index 5a817e3d6c..717cbde140 100644
--- a/lms/djangoapps/courseware/grades.py
+++ b/lms/djangoapps/courseware/grades.py
@@ -12,18 +12,23 @@ _log = logging.getLogger("mitx.courseware")
def grade_sheet(student, course, grader, student_module_cache):
"""
- This pulls a summary of all problems in the course. It returns a dictionary with two datastructures:
+ This pulls a summary of all problems in the course. It returns a dictionary
+ with two datastructures:
- - courseware_summary is a summary of all sections with problems in the course. It is organized as an array of chapters,
- each containing an array of sections, each containing an array of scores. This contains information for graded and ungraded
- problems, and is good for displaying a course summary with due dates, etc.
+ - courseware_summary is a summary of all sections with problems in the
+ course. It is organized as an array of chapters, each containing an array of
+ sections, each containing an array of scores. This contains information for
+ graded and ungraded problems, and is good for displaying a course summary
+ with due dates, etc.
- - grade_summary is the output from the course grader. More information on the format is in the docstring for CourseGrader.
+ - grade_summary is the output from the course grader. More information on
+ the format is in the docstring for CourseGrader.
Arguments:
student: A User object for the student to grade
course: An XModule containing the course to grade
- student_module_cache: A StudentModuleCache initialized with all instance_modules for the student
+ student_module_cache: A StudentModuleCache initialized with all
+ instance_modules for the student
"""
totaled_scores = {}
chapters = []
@@ -51,12 +56,16 @@ def grade_sheet(student, course, grader, student_module_cache):
correct = total
if not total > 0:
- #We simply cannot grade a problem that is 12/0, because we might need it as a percentage
+ #We simply cannot grade a problem that is 12/0, because we
+ #might need it as a percentage
graded = False
- scores.append(Score(correct, total, graded, module.metadata.get('display_name')))
+ scores.append(Score(correct, total, graded,
+ module.metadata.get('display_name')))
+
+ section_total, graded_total = graders.aggregate_scores(
+ scores, s.metadata.get('display_name'))
- section_total, graded_total = graders.aggregate_scores(scores, s.metadata.get('display_name'))
#Add the graded total to totaled_scores
format = s.metadata.get('format', "")
if format and graded_total.possible > 0:
@@ -65,7 +74,8 @@ def grade_sheet(student, course, grader, student_module_cache):
totaled_scores[format] = format_scores
sections.append({
- 'section': s.metadata.get('display_name'),
+ 'display_name': s.metadata.get('display_name'),
+ 'url_name': s.metadata.get('url_name'),
'scores': scores,
'section_total': section_total,
'format': format,
@@ -74,7 +84,8 @@ def grade_sheet(student, course, grader, student_module_cache):
})
chapters.append({'course': course.metadata.get('display_name'),
- 'chapter': c.metadata.get('display_name'),
+ 'display_name': c.metadata.get('display_name'),
+ 'url_name': c.metadata.get('url_name'),
'sections': sections})
grade_summary = grader.grade(totaled_scores)
diff --git a/lms/djangoapps/courseware/module_render.py b/lms/djangoapps/courseware/module_render.py
index 91d4efa651..4699ed50a4 100644
--- a/lms/djangoapps/courseware/module_render.py
+++ b/lms/djangoapps/courseware/module_render.py
@@ -35,10 +35,12 @@ def toc_for_course(user, request, course, active_chapter, active_section):
Create a table of contents from the module store
Return format:
- [ {'name': name, 'sections': SECTIONS, 'active': bool}, ... ]
+ [ {'display_name': name, 'url_name': url_name,
+ 'sections': SECTIONS, 'active': bool}, ... ]
where SECTIONS is a list
- [ {'name': name, 'format': format, 'due': due, 'active' : bool}, ...]
+ [ {'display_name': name, 'url_name': url_name,
+ 'format': format, 'due': due, 'active' : bool}, ...]
active is set for the section and chapter corresponding to the passed
parameters. Everything else comes from the xml, or defaults to "".
@@ -59,12 +61,14 @@ def toc_for_course(user, request, course, active_chapter, active_section):
hide_from_toc = section.metadata.get('hide_from_toc', 'false').lower() == 'true'
if not hide_from_toc:
- sections.append({'name': section.metadata.get('display_name'),
+ sections.append({'display_name': section.metadata.get('display_name'),
+ 'url_name': section.metadata.get('url_name'),
'format': section.metadata.get('format', ''),
'due': section.metadata.get('due', ''),
'active': active})
- chapters.append({'name': chapter.metadata.get('display_name'),
+ chapters.append({'display_name': chapter.metadata.get('display_name'),
+ 'url_name': chapter.metadata.get('url_name'),
'sections': sections,
'active': chapter.metadata.get('display_name') == active_chapter})
return chapters
@@ -76,8 +80,8 @@ def get_section(course_module, chapter, section):
or None if this doesn't specify a valid section
course: Course url
- chapter: Chapter name
- section: Section name
+ chapter: Chapter url_name
+ section: Section url_name
"""
if course_module is None:
@@ -85,7 +89,7 @@ def get_section(course_module, chapter, section):
chapter_module = None
for _chapter in course_module.get_children():
- if _chapter.metadata.get('display_name') == chapter:
+ if _chapter.metadata.get('url_name') == chapter:
chapter_module = _chapter
break
@@ -94,7 +98,7 @@ def get_section(course_module, chapter, section):
section_module = None
for _section in chapter_module.get_children():
- if _section.metadata.get('display_name') == section:
+ if _section.metadata.get('url_name') == section:
section_module = _section
break
@@ -141,12 +145,12 @@ def get_module(user, request, location, student_module_cache, position=None):
# Setup system context for module instance
ajax_url = settings.MITX_ROOT_URL + '/modx/' + descriptor.location.url() + '/'
- # Fully qualified callback URL for external queueing system
- xqueue_callback_url = (request.build_absolute_uri('/') + settings.MITX_ROOT_URL +
- 'xqueue/' + str(user.id) + '/' + descriptor.location.url() + '/' +
+ # Fully qualified callback URL for external queueing system
+ xqueue_callback_url = (request.build_absolute_uri('/') + settings.MITX_ROOT_URL +
+ 'xqueue/' + str(user.id) + '/' + descriptor.location.url() + '/' +
'score_update')
- # Default queuename is course-specific and is derived from the course that
+ # Default queuename is course-specific and is derived from the course that
# contains the current module.
# TODO: Queuename should be derived from 'course_settings.json' of each course
xqueue_default_queuename = descriptor.location.org + '-' + descriptor.location.course
diff --git a/lms/djangoapps/courseware/views.py b/lms/djangoapps/courseware/views.py
index 2b55b48caf..18b710e108 100644
--- a/lms/djangoapps/courseware/views.py
+++ b/lms/djangoapps/courseware/views.py
@@ -54,11 +54,6 @@ def user_groups(user):
return group_names
-def format_url_params(params):
- return [urllib.quote(string.replace(' ', '_'))
- if string is not None else None
- for string in params]
-
@ensure_csrf_cookie
@cache_if_anonymous
@@ -124,7 +119,6 @@ def profile(request, course_id, student_id=None):
'language': user_info.language,
'email': student.email,
'course': course,
- 'format_url_params': format_url_params,
'csrf': csrf(request)['csrf_token']
}
context.update(grades.grade_sheet(student, course_module, course.grader, student_module_cache))
@@ -138,9 +132,9 @@ def render_accordion(request, course, chapter, section):
If chapter and section are '' or None, renders a default accordion.
- Returns (initialization_javascript, content)'''
+ Returns the html string'''
- # TODO (cpennington): do the right thing with courses
+ # grab the table of contents
toc = toc_for_course(request.user, request, course, chapter, section)
active_chapter = 1
@@ -152,7 +146,6 @@ def render_accordion(request, course, chapter, section):
('toc', toc),
('course_name', course.title),
('course_id', course.id),
- ('format_url_params', format_url_params),
('csrf', csrf(request)['csrf_token'])] + template_imports.items())
return render_to_string('accordion.html', context)
@@ -169,9 +162,9 @@ def index(request, course_id, chapter=None, section=None,
Arguments:
- request : HTTP request
- - course : coursename (str)
- - chapter : chapter name (str)
- - section : section name (str)
+ - course_id : course id (str: ORG/course/URL_NAME)
+ - chapter : chapter url_name (str)
+ - section : section url_name (str)
- position : position in module, eg of module (str)
Returns:
@@ -180,16 +173,6 @@ def index(request, course_id, chapter=None, section=None,
'''
course = check_course(course_id)
- def clean(s):
- ''' Fixes URLs -- we convert spaces to _ in URLs to prevent
- funny encoding characters and keep the URLs readable. This undoes
- that transformation.
- '''
- return s.replace('_', ' ') if s is not None else None
-
- chapter = clean(chapter)
- section = clean(section)
-
try:
context = {
'csrf': csrf(request)['csrf_token'],
@@ -202,8 +185,6 @@ def index(request, course_id, chapter=None, section=None,
look_for_module = chapter is not None and section is not None
if look_for_module:
- # TODO (cpennington): Pass the right course in here
-
section_descriptor = get_section(course, chapter, section)
if section_descriptor is not None:
student_module_cache = StudentModuleCache(request.user,
diff --git a/lms/templates/accordion.html b/lms/templates/accordion.html
index defb424a29..353b83db70 100644
--- a/lms/templates/accordion.html
+++ b/lms/templates/accordion.html
@@ -1,13 +1,13 @@
<%! from django.core.urlresolvers import reverse %>
<%def name="make_chapter(chapter)">
-