Remove semantically meaningless sections from course.xml by moving their…

Remove semantically meaningless sections from course.xml by moving their attributes onto the contained element. If there is more than one contained element, turn the section into a sequence. Also handles includes

cms/djangoapps/contentstore/management/commands/import.py

@@ -6,9 +6,10 @@ from django.core.management.base import BaseCommand
 from keystore.django import keystore
 from raw_module import RawDescriptor
 from lxml import etree
+from fs.osfs import OSFS
 
 from path import path
-from x_module import XModuleDescriptor, DescriptorSystem
+from x_module import XModuleDescriptor, XMLParsingSystem
 
 unnamed_modules = 0
 
@@ -24,27 +25,29 @@ class Command(BaseCommand):
         data_dir = path(data_dir)
         with open(data_dir / "course.xml") as course_file:
 
-            system = DescriptorSystem(keystore().get_item)
-
-            def process_xml(xml):
-                try:
-                    xml_data = etree.fromstring(xml)
-                except:
-                    print xml
-                    raise
-                if not xml_data.get('name'):
-                    global unnamed_modules
-                    unnamed_modules += 1
-                    xml_data.set('name', 'Unnamed module %d' % unnamed_modules)
-
-
-                module = XModuleDescriptor.load_from_xml(etree.tostring(xml_data), system, org, course, RawDescriptor)
-                keystore().create_item(module.url)
-                if 'data' in module.definition:
-                    keystore().update_item(module.url, module.definition['data'])
-                if 'children' in module.definition:
-                    keystore().update_children(module.url, module.definition['children'])
-                return module.url
-
-            system.process_xml = process_xml
-            system.process_xml(course_file.read())
+            class ImportSystem(XMLParsingSystem):
+                def __init__(self):
+                    self.load_item = keystore().get_item
+                    self.fs = OSFS(data_dir)
+
+                def process_xml(self, xml):
+                    try:
+                        xml_data = etree.fromstring(xml)
+                    except:
+                        print xml
+                        raise
+                    if not xml_data.get('name'):
+                        global unnamed_modules
+                        unnamed_modules += 1
+                        xml_data.set('name', 'Unnamed module %d' % unnamed_modules)
+
+
+                    module = XModuleDescriptor.load_from_xml(etree.tostring(xml_data), self, org, course, RawDescriptor)
+                    keystore().create_item(module.url)
+                    if 'data' in module.definition:
+                        keystore().update_item(module.url, module.definition['data'])
+                    if 'children' in module.definition:
+                        keystore().update_children(module.url, module.definition['children'])
+                    return module
+
+            ImportSystem().process_xml(course_file.read())

common/lib/xmodule/html_module.py

@@ -19,6 +19,28 @@ class HtmlModuleDescriptor(MakoModuleDescriptor):
     js = {'coffee': [resource_string(__name__, 'js/module/html.coffee')]}
     js_module = 'HTML'
 
+    @classmethod
+    def from_xml(cls, xml_data, system, org=None, course=None):
+        """
+        Creates an instance of this descriptor from the supplied xml_data.
+        This may be overridden by subclasses
+
+        xml_data: A string of xml that will be translated into data and children for
+            this module
+        system: An XModuleSystem for interacting with external resources
+        org and course are optional strings that will be used in the generated modules
+            url identifiers
+        """
+        xml_object = etree.fromstring(xml_data)
+        return cls(
+            system,
+            definition={'data': {'text': xml_data}},
+            location=['i4x',
+                      org,
+                      course,
+                      xml_object.tag,
+                      xml_object.get('name')]
+        )
 
 class Module(XModule):
     id_attribute = 'filename'

common/lib/xmodule/seq_module.py

@@ -123,7 +123,7 @@ class SequenceDescriptor(MakoModuleDescriptor):
         xml_object = etree.fromstring(xml_data)
 
         children = [
-            system.process_xml(etree.tostring(child_module))
+            system.process_xml(etree.tostring(child_module)).url
             for child_module in xml_object
         ]
 

common/lib/xmodule/setup.py

@@ -13,6 +13,7 @@ setup(
     # for a description of entry_points
     entry_points={
         'xmodule.v1': [
+            "section = translation_module:SemanticSectionDescriptor",
             "chapter = seq_module:SequenceDescriptor",
             "course = seq_module:SequenceDescriptor",
             "html = html_module:HtmlModuleDescriptor",

common/lib/xmodule/translation_module.py

+"""
+These modules exist to translate old format XML into newer, semantic forms
+"""
+from x_module import XModuleDescriptor
+from lxml import etree
+from functools import wraps
+import logging
+
+log = logging.getLogger(__name__)
+
+
+def process_includes(fn):
+    """
+    Wraps a XModuleDescriptor.from_xml method, and modifies xml_data to replace
+    any immediate child <include> items with the contents of the file that they are
+    supposed to include
+    """
+    @wraps(fn)
+    def from_xml(cls, xml_data, system, org=None, course=None):
+        xml_object = etree.fromstring(xml_data)
+        next_include = xml_object.find('include')
+        while next_include is not None:
+            file = next_include.get('file')
+            if file is not None:
+                try:
+                    ifp = system.fs.open(file)
+                except Exception:
+                    log.exception('Error in problem xml include: %s' % (etree.tostring(next_include, pretty_print=True)))
+                    log.exception('Cannot find file %s in %s' % (file, dir))
+                    raise
+                try:
+                    # read in and convert to XML
+                    incxml = etree.XML(ifp.read())
+                except Exception:
+                    log.exception('Error in problem xml include: %s' % (etree.tostring(next_include, pretty_print=True)))
+                    log.exception('Cannot parse XML in %s' % (file))
+                    raise
+                # insert  new XML into tree in place of inlcude
+                parent = next_include.getparent()
+                parent.insert(parent.index(next_include), incxml)
+            parent.remove(next_include)
+
+            next_include = xml_object.find('include')
+        return fn(cls, etree.tostring(xml_object), system, org, course)
+    return from_xml
+
+
+class SemanticSectionDescriptor(XModuleDescriptor):
+    @classmethod
+    @process_includes
+    def from_xml(cls, xml_data, system, org=None, course=None):
+        """
+        Removes sections single child elements in favor of just embedding the child element
+
+        """
+        xml_object = etree.fromstring(xml_data)
+
+        if len(xml_object) == 1:
+            for (key, val) in xml_object.items():
+                if key == 'format':
+                    continue
+                xml_object[0].set(key, val)
+
+            return system.process_xml(etree.tostring(xml_object[0]))
+        else:
+            xml_object.tag = 'sequence'
+            return system.process_xml(etree.tostring(xml_object))

common/lib/xmodule/x_module.py

@@ -213,6 +213,7 @@ class XModuleDescriptor(Plugin):
         on the contents of xml_data.
 
         xml_data must be a string containing valid xml
+        system is an XMLParsingSystem
         org and course are optional strings that will be used in the generated modules
             url identifiers
         """
@@ -230,7 +231,7 @@ class XModuleDescriptor(Plugin):
 
         xml_data: A string of xml that will be translated into data and children for
             this module
-        system: An XModuleSystem for interacting with external resources
+        system is an XMLParsingSystem
         org and course are optional strings that will be used in the generated modules
             url identifiers
         """
@@ -330,11 +331,20 @@ class XModuleDescriptor(Plugin):
 
 
 class DescriptorSystem(object):
-    def __init__(self, load_item, process_xml=None):
+    def __init__(self, load_item):
         """
         load_item: Takes a Location and returns an XModuleDescriptor
-        process_xml: Takes an xml string, and returns the url of the XModuleDescriptor created from that xml
         """
 
         self.load_item = load_item
+
+
+class XMLParsingSystem(DescriptorSystem):
+    def __init__(self, load_item, process_xml, fs):
+        """
+        process_xml: Takes an xml string, and returns the the XModuleDescriptor created from that xml
+        fs: A Filesystem object that contains all of the xml resources needed to parse
+            the course
+        """
         self.process_xml = process_xml
+        self.fs = fs