Enable (and test) xml export of pure XBlocks

[LMS-179] [LMS-209] [LMS-1345]
2014-01-17 15:27:12 -05:00
parent 69097e7b4a
commit b0bed170de
12 changed files with 131 additions and 85 deletions
--- a/common/lib/xmodule/xmodule/abtest_module.py
+++ b/common/lib/xmodule/xmodule/abtest_module.py
@@ -149,7 +149,7 @@ class ABTestDescriptor(ABTestFields, RawDescriptor, XmlDescriptor):

            for child_loc in group:
                child = self.system.load_item(child_loc)
-                group_elem.append(etree.fromstring(child.export_to_xml(resource_fs)))
+                self.runtime.add_block_as_child_node(child, group_elem)

        return xml_object

--- a/common/lib/xmodule/xmodule/conditional_module.py
+++ b/common/lib/xmodule/xmodule/conditional_module.py
@@ -222,9 +222,9 @@ class ConditionalDescriptor(ConditionalFields, SequenceDescriptor):
        show_tag_list = []
        for child in xml_object:
            if child.tag == 'show':
-                location = ConditionalDescriptor.parse_sources(child, system)
-                children.extend(location)
-                show_tag_list.extend(location.url())  # pylint: disable=no-member
+                locations = ConditionalDescriptor.parse_sources(child, system)
+                children.extend(locations)
+                show_tag_list.extend(location.url() for location in locations)  # pylint: disable=no-member
            else:
                try:
                    descriptor = system.process_xml(etree.tostring(child))
@@ -244,6 +244,5 @@ class ConditionalDescriptor(ConditionalFields, SequenceDescriptor):
                    tag_name='show', sources=location)
                xml_object.append(etree.fromstring(show_str))
            else:
-                xml_object.append(
-                    etree.fromstring(child.export_to_xml(resource_fs)))
+                self.runtime.add_block_as_child_node(child, xml_object)
        return xml_object
--- a/common/lib/xmodule/xmodule/crowdsource_hinter.py
+++ b/common/lib/xmodule/xmodule/crowdsource_hinter.py
@@ -400,6 +400,5 @@ class CrowdsourceHinterDescriptor(CrowdsourceHinterFields, RawDescriptor):
    def definition_to_xml(self, resource_fs):
        xml_object = etree.Element('crowdsource_hinter')
        for child in self.get_children():
-            xml_object.append(
-                etree.fromstring(child.export_to_xml(resource_fs)))
+            self.runtime.add_block_as_child_node(child, xml_object)
        return xml_object
--- a/common/lib/xmodule/xmodule/modulestore/xml_exporter.py
+++ b/common/lib/xmodule/xmodule/modulestore/xml_exporter.py
@@ -3,6 +3,7 @@ Methods for exporting course data to XML
 """

 import logging
+import lxml.etree
 from xmodule.modulestore import Location
 from xmodule.modulestore.inheritance import own_metadata
 from fs.osfs import OSFS
@@ -57,11 +58,13 @@ def export_to_xml(modulestore, contentstore, course_location, root_dir, course_d
    course = modulestore.get_course(course_id)

    fs = OSFS(root_dir)
-    export_fs = fs.makeopendir(course_dir)
+    export_fs = course.runtime.export_fs = fs.makeopendir(course_dir)
+
+    root = lxml.etree.Element('unknown')
+    course.add_xml_to_node(root)

-    xml = course.export_to_xml(export_fs)
    with export_fs.open('course.xml', 'w') as course_xml:
-        course_xml.write(xml)
+        lxml.etree.ElementTree(root).write(course_xml)

    # export the static assets
    policies_dir = export_fs.makeopendir('policies')
@@ -112,7 +115,9 @@ def export_to_xml(modulestore, contentstore, course_location, root_dir, course_d
                    sequential = modulestore.get_item(Location(parent_locs[0]))
                    index = sequential.children.index(draft_vertical.location.url())
                    draft_vertical.xml_attributes['index_in_children_list'] = str(index)
-                    draft_vertical.export_to_xml(draft_course_dir)
+                    draft_vertical.runtime.export_fs = draft_course_dir
+                    node = lxml.etree.Element('unknown')
+                    draft_vertical.add_xml_to_node(node)


 def export_extra_content(export_fs, modulestore, course_id, course_location, category_type, dirname, file_suffix=''):
--- a/common/lib/xmodule/xmodule/randomize_module.py
+++ b/common/lib/xmodule/xmodule/randomize_module.py
@@ -100,8 +100,7 @@ class RandomizeDescriptor(RandomizeFields, SequenceDescriptor):

        xml_object = etree.Element('randomize')
        for child in self.get_children():
-            xml_object.append(
-                etree.fromstring(child.export_to_xml(resource_fs)))
+            self.runtime.add_block_as_child_node(child, xml_object)
        return xml_object

    def has_dynamic_children(self):
--- a/common/lib/xmodule/xmodule/seq_module.py
+++ b/common/lib/xmodule/xmodule/seq_module.py
@@ -150,6 +150,5 @@ class SequenceDescriptor(SequenceFields, MakoModuleDescriptor, XmlDescriptor):
    def definition_to_xml(self, resource_fs):
        xml_object = etree.Element('sequential')
        for child in self.get_children():
-            xml_object.append(
-                etree.fromstring(child.export_to_xml(resource_fs)))
+            self.runtime.add_block_as_child_node(child, xml_object)
        return xml_object
--- a/common/lib/xmodule/xmodule/tests/test_export.py
+++ b/common/lib/xmodule/xmodule/tests/test_export.py
@@ -2,19 +2,25 @@
 Tests of XML export
 """

-from datetime import datetime, timedelta, tzinfo
-from tempfile import mkdtemp
-import unittest
-import shutil
-from textwrap import dedent
+import ddt
+import lxml.etree
 import mock
-
+import os
 import pytz
+import shutil
+import tarfile
+import unittest
+import uuid
+
+from datetime import datetime, timedelta, tzinfo
 from fs.osfs import OSFS
 from path import path
-import uuid
-import tarfile
-import os
+from tempfile import mkdtemp
+from textwrap import dedent
+
+from xblock.core import XBlock
+from xblock.fields import String, Scope, Integer
+from xblock.test.tools import blocks_are_equivalent

 from xmodule.modulestore import Location
 from xmodule.modulestore.xml import XMLModuleStore
@@ -23,6 +29,7 @@ from xmodule.modulestore.xml_exporter import (
 )
 from xmodule.tests import DATA_DIR
 from xmodule.tests.helpers import directories_equal
+from xmodule.x_module import XModuleMixin


 def strip_filenames(descriptor):
@@ -43,6 +50,16 @@ def strip_filenames(descriptor):
    descriptor.save()


+class PureXBlock(XBlock):
+
+    """Class for testing pure XBlocks."""
+
+    has_children = True
+    field1 = String(default="something", scope=Scope.user_state)
+    field2 = Integer(scope=Scope.user_state)
+
+
+@ddt.ddt
 class RoundTripTestCase(unittest.TestCase):
    """
    Check that our test courses roundtrip properly.
@@ -51,8 +68,25 @@ class RoundTripTestCase(unittest.TestCase):
    Thus we make sure that export and import work properly.
    """

+
+    def setUp(self):
+        self.maxDiff = None
+        self.temp_dir = mkdtemp()
+        self.addCleanup(shutil.rmtree, self.temp_dir)
+
    @mock.patch('xmodule.course_module.requests.get')
-    def check_export_roundtrip(self, data_dir, course_dir, mock_get):
+    @ddt.data(
+        "toy",
+        "simple",
+        "conditional_and_poll",
+        "self_assessment",
+        "graphic_slider_tool",
+        "test_exam_registration",
+        "word_cloud",
+        "pure_xblock",
+    )
+    @XBlock.register_temp_plugin(PureXBlock, 'pure')
+    def test_export_roundtrip(self, course_dir, mock_get):

        # Patch network calls to retrieve the textbook TOC
        mock_get.return_value.text = dedent("""
@@ -64,11 +98,11 @@ class RoundTripTestCase(unittest.TestCase):
        root_dir = path(self.temp_dir)
        print("Copying test course to temp dir {0}".format(root_dir))

-        data_dir = path(data_dir)
+        data_dir = path(DATA_DIR)
        shutil.copytree(data_dir / course_dir, root_dir / course_dir)

        print("Starting import")
-        initial_import = XMLModuleStore(root_dir, course_dirs=[course_dir])
+        initial_import = XMLModuleStore(root_dir, course_dirs=[course_dir], xblock_mixins=(XModuleMixin,))

        courses = initial_import.get_courses()
        self.assertEquals(len(courses), 1)
@@ -78,14 +112,15 @@ class RoundTripTestCase(unittest.TestCase):
        # will still be there.
        print("Starting export")
        fs = OSFS(root_dir)
-        export_fs = fs.makeopendir(course_dir)
+        initial_course.runtime.export_fs = fs.makeopendir(course_dir)
+        root = lxml.etree.Element('root')

-        xml = initial_course.export_to_xml(export_fs)
-        with export_fs.open('course.xml', 'w') as course_xml:
-            course_xml.write(xml)
+        initial_course.add_xml_to_node(root)
+        with initial_course.runtime.export_fs.open('course.xml', 'w') as course_xml:
+            lxml.etree.ElementTree(root).write(course_xml)

        print("Starting second import")
-        second_import = XMLModuleStore(root_dir, course_dirs=[course_dir])
+        second_import = XMLModuleStore(root_dir, course_dirs=[course_dir], xblock_mixins=(XModuleMixin,))

        courses2 = second_import.get_courses()
        self.assertEquals(len(courses2), 1)
@@ -98,48 +133,24 @@ class RoundTripTestCase(unittest.TestCase):
        strip_filenames(initial_course)
        strip_filenames(exported_course)

-        self.assertEquals(initial_course, exported_course)
+        self.assertTrue(blocks_are_equivalent(initial_course, exported_course))
        self.assertEquals(initial_course.id, exported_course.id)
        course_id = initial_course.id

        print("Checking key equality")
-        self.assertEquals(sorted(initial_import.modules[course_id].keys()),
-                          sorted(second_import.modules[course_id].keys()))
+        self.assertItemsEqual(
+            initial_import.modules[course_id].keys(),
+            second_import.modules[course_id].keys()
+        )

        print("Checking module equality")
        for location in initial_import.modules[course_id].keys():
            print("Checking", location)
-            self.assertEquals(initial_import.modules[course_id][location],
-                              second_import.modules[course_id][location])
+            self.assertTrue(blocks_are_equivalent(
+                initial_import.modules[course_id][location],
+                second_import.modules[course_id][location]
+            ))

-    def setUp(self):
-        self.maxDiff = None
-        self.temp_dir = mkdtemp()
-        self.addCleanup(shutil.rmtree, self.temp_dir)
-
-    def test_toy_roundtrip(self):
-        self.check_export_roundtrip(DATA_DIR, "toy")
-
-    def test_simple_roundtrip(self):
-        self.check_export_roundtrip(DATA_DIR, "simple")
-
-    def test_conditional_and_poll_roundtrip(self):
-        self.check_export_roundtrip(DATA_DIR, "conditional_and_poll")
-
-    def test_selfassessment_roundtrip(self):
-        #Test selfassessment xmodule to see if it exports correctly
-        self.check_export_roundtrip(DATA_DIR, "self_assessment")
-
-    def test_graphicslidertool_roundtrip(self):
-        #Test graphicslidertool xmodule to see if it exports correctly
-        self.check_export_roundtrip(DATA_DIR, "graphic_slider_tool")
-
-    def test_exam_registration_roundtrip(self):
-        # Test exam_registration xmodule to see if it exports correctly
-        self.check_export_roundtrip(DATA_DIR, "test_exam_registration")
-
-    def test_word_cloud_roundtrip(self):
-        self.check_export_roundtrip(DATA_DIR, "word_cloud")


 class TestEdxJsonEncoder(unittest.TestCase):
--- a/common/lib/xmodule/xmodule/tests/test_import.py
+++ b/common/lib/xmodule/xmodule/tests/test_import.py
@@ -158,9 +158,9 @@ class ImportTestCase(BaseCourseTestCase):
        system = self.get_system()
        descriptor = system.process_xml(bad_xml)

-        resource_fs = None
-        tag_xml = descriptor.export_to_xml(resource_fs)
-        re_import_descriptor = system.process_xml(tag_xml)
+        node = etree.Element('unknown')
+        descriptor.add_xml_to_node(node)
+        re_import_descriptor = system.process_xml(etree.tostring(node))

        self.assertEqual(re_import_descriptor.__class__.__name__, 'ErrorDescriptorWithMixins')

@@ -182,12 +182,11 @@ class ImportTestCase(BaseCourseTestCase):
        descriptor = system.process_xml(xml_str_in)

        # export it
-        resource_fs = None
-        xml_str_out = descriptor.export_to_xml(resource_fs)
+        node = etree.Element('unknown')
+        descriptor.add_xml_to_node(node)

        # Now make sure the exported xml is a sequential
-        xml_out = etree.fromstring(xml_str_out)
-        self.assertEqual(xml_out.tag, 'sequential')
+        self.assertEqual(node.tag, 'sequential')

    def test_metadata_import_export(self):
        """Two checks:
@@ -221,19 +220,19 @@ class ImportTestCase(BaseCourseTestCase):
        )

        # Now export and check things
-        resource_fs = MemoryFS()
-        exported_xml = descriptor.export_to_xml(resource_fs)
+        descriptor.runtime.export_fs = MemoryFS()
+        node = etree.Element('unknown')
+        descriptor.add_xml_to_node(node)

        # Check that the exported xml is just a pointer
-        print("Exported xml:", exported_xml)
-        pointer = etree.fromstring(exported_xml)
-        self.assertTrue(is_pointer_tag(pointer))
+        print("Exported xml:", etree.tostring(node))
+        self.assertTrue(is_pointer_tag(node))
        # but it's a special case course pointer
-        self.assertEqual(pointer.attrib['course'], COURSE)
-        self.assertEqual(pointer.attrib['org'], ORG)
+        self.assertEqual(node.attrib['course'], COURSE)
+        self.assertEqual(node.attrib['org'], ORG)

        # Does the course still have unicorns?
-        with resource_fs.open('course/{url_name}.xml'.format(url_name=url_name)) as f:
+        with descriptor.runtime.export_fs.open('course/{url_name}.xml'.format(url_name=url_name)) as f:
            course_xml = etree.fromstring(f.read())

        self.assertEqual(course_xml.attrib['unicorn'], 'purple')
@@ -247,7 +246,7 @@ class ImportTestCase(BaseCourseTestCase):

        # Does the chapter tag now have a due attribute?
        # hardcoded path to child
-        with resource_fs.open('chapter/ch.xml') as f:
+        with descriptor.runtime.export_fs.open('chapter/ch.xml') as f:
            chapter_xml = etree.fromstring(f.read())
        self.assertEqual(chapter_xml.tag, 'chapter')
        self.assertFalse('due' in chapter_xml.attrib)
--- a/common/lib/xmodule/xmodule/x_module.py
+++ b/common/lib/xmodule/xmodule/x_module.py
@@ -222,6 +222,7 @@ class XModuleMixin(XBlockMixin):
            for child_loc in self.children:
                try:
                    child = self.runtime.get_block(child_loc)
+                    child.runtime.export_fs = self.runtime.export_fs
                except ItemNotFoundError:
                    log.exception(u'Unable to load item {loc}, skipping'.format(loc=child_loc))
                    continue
@@ -685,6 +686,21 @@ class XModuleDescriptor(XModuleMixin, HTMLSnippet, ResourceTemplates, XBlock):
        """
        raise NotImplementedError('Modules must implement from_xml to be parsable from xml')

+    def add_xml_to_node(self, node):
+        """
+        Export this :class:`XModuleDescriptor` as XML, by setting attributes on the provided
+        `node`.
+        """
+        xml_string = self.export_to_xml(self.runtime.export_fs)
+        exported_node = etree.fromstring(xml_string)
+        node.tag = exported_node.tag
+        node.text = exported_node.text
+        node.tail = exported_node.tail
+        for key, value in exported_node.items():
+            node.set(key, value)
+
+        node.extend(list(exported_node))
+
    def export_to_xml(self, resource_fs):
        """
        Returns an xml string representing this module, and all modules
@@ -926,6 +942,9 @@ class DescriptorSystem(ConfigurableFragmentWrapper, Runtime):  # pylint: disable
        """
        super(DescriptorSystem, self).__init__(**kwargs)

+        # This is used by XModules to write out separate files during xml export
+        self.export_fs = None
+
        self.load_item = load_item
        self.resources_fs = resources_fs
        self.error_tracker = error_tracker
@@ -996,6 +1015,11 @@ class DescriptorSystem(ConfigurableFragmentWrapper, Runtime):  # pylint: disable
    def publish(self, block, event):
        raise NotImplementedError("edX Platform doesn't currently implement XBlock publish")

+    def add_block_as_child_node(self, block, node):
+        child = etree.SubElement(node, "unknown")
+        child.set('url_name', block.url_name)
+        block.add_xml_to_node(child)
+

 class XMLParsingSystem(DescriptorSystem):
    def __init__(self, process_xml, **kwargs):
--- a/common/test/data/pure_xblock/course.xml
+++ b/common/test/data/pure_xblock/course.xml
@@ -0,0 +1,8 @@
+<course org="edX" course="pure_xblock" url_name="2012_Fall">
+    <pure field1="a.field1" url_name="pure0">
+        <vertical url_name="vert0">
+            <pure field2="10" url_name="pure1"/>
+            <pure field2="20" url_name="pure2"/>
+        </vertical>
+    </pure>
+</course>
--- a/lms/djangoapps/courseware/management/commands/clean_xml.py
+++ b/lms/djangoapps/courseware/management/commands/clean_xml.py
@@ -1,3 +1,4 @@
+import lxml.etree
 import os
 import sys
 import traceback
@@ -30,9 +31,11 @@ def export(course, export_dir):
               '  May clobber/confuse things'.format(dir=export_dir))

    try:
-        xml = course.export_to_xml(fs)
+        course.runtime.export_fs = fs
+        root = lxml.etree.Element('root')
+        course.add_xml_to_node(root)
        with fs.open('course.xml', mode='w') as f:
-            f.write(xml)
+            root.write(f)

        return True
    except:
--- a/requirements/edx/github.txt
+++ b/requirements/edx/github.txt
@@ -15,7 +15,7 @@
 -e git+https://github.com/eventbrite/zendesk.git@d53fe0e81b623f084e91776bcf6369f8b7b63879#egg=zendesk

 # Our libraries:
-e git+https://github.com/edx/XBlock.git@de92d3bf798699a6bbd06b54012ef15934c41ac0#egg=XBlock
+-e git+https://github.com/edx/XBlock.git@3830ee50015b460fad63ff3b71f77bf1a2684195#egg=XBlock
 -e git+https://github.com/edx/codejail.git@e3d98f9455#egg=codejail
 -e git+https://github.com/edx/diff-cover.git@v0.2.9#egg=diff_cover
 -e git+https://github.com/edx/js-test-tool.git@v0.1.5#egg=js_test_tool