diff --git a/common/lib/xmodule/xmodule/error_module.py b/common/lib/xmodule/xmodule/error_module.py index ecc90873b9..cbd805396f 100644 --- a/common/lib/xmodule/xmodule/error_module.py +++ b/common/lib/xmodule/xmodule/error_module.py @@ -1,5 +1,7 @@ -import sys import logging +import random +import string +import sys from pkg_resources import resource_string from lxml import etree @@ -35,7 +37,8 @@ class ErrorDescriptor(EditingDescriptor): 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. + Does not require that xml_data be parseable--just stores it and exports + as-is if not. Takes an extra, optional, parameter--the error that caused an issue. (should be a string, or convert usefully into one). @@ -45,6 +48,14 @@ class ErrorDescriptor(EditingDescriptor): definition = {'data': inner} inner['error_msg'] = str(error_msg) + # Pick a unique (random) url_name. + # NOTE: We could try to pull out the url_name of the errored descriptor, + # but url_names aren't guaranteed to be unique between descriptor types, + # and ErrorDescriptor can wrap any type. When the wrapped module is fixed, + # it will be written out with the original url_name. + chars = string.ascii_uppercase + string.ascii_lowercase + string.digits + url_name = ''.join(random.choice(chars) for i in range(16)) + try: # If this is already an error tag, don't want to re-wrap it. xml_obj = etree.fromstring(xml_data) @@ -63,7 +74,7 @@ class ErrorDescriptor(EditingDescriptor): 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'] + location = ['i4x', org, course, 'error', url_name] metadata = {} # stays in the xml_data return cls(system, definition, location=location, metadata=metadata) diff --git a/common/lib/xmodule/xmodule/tests/test_import.py b/common/lib/xmodule/xmodule/tests/test_import.py index af7464cfdc..407057a4ab 100644 --- a/common/lib/xmodule/xmodule/tests/test_import.py +++ b/common/lib/xmodule/xmodule/tests/test_import.py @@ -47,8 +47,6 @@ class DummySystem(XMLParsingSystem): raise Exception("Shouldn't be called") - - class ImportTestCase(unittest.TestCase): '''Make sure module imports work properly, including for malformed inputs''' @staticmethod @@ -57,10 +55,9 @@ class ImportTestCase(unittest.TestCase): return DummySystem() def test_fallback(self): - '''Make sure that malformed xml loads as an ErrorDescriptor.''' + '''Check that malformed xml loads as an ErrorDescriptor.''' bad_xml = '''''' - system = self.get_system() descriptor = XModuleDescriptor.load_from_xml(bad_xml, system, 'org', 'course', @@ -69,6 +66,22 @@ class ImportTestCase(unittest.TestCase): self.assertEqual(descriptor.__class__.__name__, 'ErrorDescriptor') + + def test_unique_url_names(self): + '''Check that each error gets its very own url_name''' + bad_xml = '''''' + bad_xml2 = '''''' + system = self.get_system() + + descriptor1 = XModuleDescriptor.load_from_xml(bad_xml, system, 'org', + 'course', None) + + descriptor2 = XModuleDescriptor.load_from_xml(bad_xml2, system, 'org', + 'course', None) + + self.assertNotEqual(descriptor1.location, descriptor2.location) + + def test_reimport(self): '''Make sure an already-exported error xml tag loads properly'''