Clean up customtag handling

* load the template at descriptor init
* make it  export inline, not as a pointer
* update import/export test to do export to initial directory

common/lib/xmodule/xmodule/template_module.py

@@ -30,22 +30,19 @@ class CustomTagModule(XModule):
                  instance_state=None, shared_state=None, **kwargs):
         XModule.__init__(self, system, location, definition, descriptor,
                          instance_state, shared_state, **kwargs)
-        self.html = definition['html']
 
     def get_html(self):
-        return self.html
+        return self.descriptor.rendered_html
 
 
 class CustomTagDescriptor(RawDescriptor):
     """ Descriptor for custom tags.  Loads the template when created."""
     module_class = CustomTagModule
 
-    @classmethod
-    def definition_from_xml(cls, xml_object, system):
-        definition = RawDescriptor.definition_from_xml(xml_object, system)
-
-        # Render the template and save it.
-        xmltree = etree.fromstring(definition['data'])
+    @staticmethod
+    def render_template(system, xml_data):
+        '''Render the template, given the definition xml_data'''
+        xmltree = etree.fromstring(xml_data)
         if 'impl' in xmltree.attrib:
             template_name = xmltree.attrib['impl']
         else:
@@ -61,6 +58,18 @@ class CustomTagDescriptor(RawDescriptor):
         params = dict(xmltree.items())
         with system.resources_fs.open('custom_tags/{name}'
                                    .format(name=template_name)) as template:
-            definition['html'] = Template(template.read()).render(**params)
+            return Template(template.read()).render(**params)
+
+
+    def __init__(self, system, definition, **kwargs):
+        '''Render and save the template for this descriptor instance'''
+        super(CustomTagDescriptor, self).__init__(system, definition, **kwargs)
+        self.rendered_html = self.render_template(system, definition['data'])
+
+    def export_to_file(self):
+        """
+        Custom tags are special: since they're already pointers, we don't want
+        to export them in a file with yet another layer of indirection.
+        """
+        return False
 
-        return definition

common/lib/xmodule/xmodule/tests/test_export.py

@@ -4,6 +4,7 @@ from fs.osfs import OSFS
 from nose.tools import assert_equals, assert_true
 from path import path
 from tempfile import mkdtemp
+from shutil import copytree
 
 from xmodule.modulestore.xml import XMLModuleStore
 
@@ -40,27 +41,32 @@ def strip_filenames(descriptor):
 class RoundTripTestCase(unittest.TestCase):
     '''Check that our test courses roundtrip properly'''
     def check_export_roundtrip(self, data_dir, course_dir):
+
+        root_dir = path(mkdtemp())
+        print "Copying test course to temp dir {0}".format(root_dir)
+
+        data_dir = path(data_dir)
+        copytree(data_dir / course_dir, root_dir / course_dir)
+
         print "Starting import"
-        initial_import = XMLModuleStore(data_dir, eager=True, course_dirs=[course_dir])
+        initial_import = XMLModuleStore(root_dir, eager=True, course_dirs=[course_dir])
 
         courses = initial_import.get_courses()
         self.assertEquals(len(courses), 1)
         initial_course = courses[0]
 
+        # export to the same directory--that way things like the custom_tags/ folder
+        # will still be there.
         print "Starting export"
-        export_dir = mkdtemp()
-        print "export_dir: {0}".format(export_dir)
-        fs = OSFS(export_dir)
-        export_course_dir = 'export'
-        export_fs = fs.makeopendir(export_course_dir)
+        fs = OSFS(root_dir)
+        export_fs = fs.makeopendir(course_dir)
 
         xml = initial_course.export_to_xml(export_fs)
         with export_fs.open('course.xml', 'w') as course_xml:
             course_xml.write(xml)
 
         print "Starting second import"
-        second_import = XMLModuleStore(export_dir, eager=True,
-                                       course_dirs=[export_course_dir])
+        second_import = XMLModuleStore(root_dir, eager=True, course_dirs=[course_dir])
 
         courses2 = second_import.get_courses()
         self.assertEquals(len(courses2), 1)

common/lib/xmodule/xmodule/x_module.py

@@ -166,6 +166,10 @@ class XModule(HTMLSnippet):
             'children': is a list of Location-like values for child modules that
                 this module depends on
 
+        descriptor: the XModuleDescriptor that this module is an instance of.
+            TODO (vshnayder): remove the definition parameter and location--they
+            can come from the descriptor.
+
         instance_state: A string of serialized json that contains the state of
                 this module for current student accessing the system, or None if
                 no state has been saved
@@ -304,9 +308,9 @@ class XModuleDescriptor(Plugin, HTMLSnippet):
     """
     entry_point = "xmodule.v1"
     module_class = XModule
-    
+
     # Attributes for inpsection of the descriptor
-    stores_state = False # Indicates whether the xmodule state should be 
+    stores_state = False # Indicates whether the xmodule state should be
     # stored in a database (independent of shared state)
     has_score = False # This indicates whether the xmodule is a problem-type.
     # It should respond to max_score() and grade(). It can be graded or ungraded
@@ -677,7 +681,7 @@ class ModuleSystem(object):
         filestore - A filestore ojbect.  Defaults to an instance of OSFS based
                          at settings.DATA_DIR.
 
-        xqueue - Dict containing XqueueInterface object, as well as parameters 
+        xqueue - Dict containing XqueueInterface object, as well as parameters
                     for the specific StudentModule
 
         replace_urls - TEMPORARY - A function like static_replace.replace_urls

common/lib/xmodule/xmodule/xml_module.py

@@ -259,6 +259,15 @@ class XmlDescriptor(XModuleDescriptor):
                                                  name=name,
                                                  ext=cls.filename_extension)
 
+    def export_to_file(self):
+        """If this returns True, write the definition of this descriptor to a separate
+        file.
+
+        NOTE: Do not override this without a good reason.  It is here specifically for customtag...
+        """
+        return True
+
+
     def export_to_xml(self, resource_fs):
         """
         Returns an xml string representing this module, and all modules
@@ -295,14 +304,18 @@ class XmlDescriptor(XModuleDescriptor):
             if attr not in self.metadata_to_strip:
                 xml_object.set(attr, val_for_xml(attr))
 
-        # Write the definition to a file
-        filepath = self.__class__._format_filepath(self.category, self.url_name)
-        resource_fs.makedir(os.path.dirname(filepath), allow_recreate=True)
-        with resource_fs.open(filepath, 'w') as file:
-            file.write(etree.tostring(xml_object, pretty_print=True))
+        if self.export_to_file():
+            # Write the definition to a file
+            filepath = self.__class__._format_filepath(self.category, self.url_name)
+            resource_fs.makedir(os.path.dirname(filepath), allow_recreate=True)
+            with resource_fs.open(filepath, 'w') as file:
+                file.write(etree.tostring(xml_object, pretty_print=True))
+
+            # And return just a pointer with the category and filename.
+            record_object = etree.Element(self.category)
+        else:
+            record_object = xml_object
 
-        # And return just a pointer with the category and filename.
-        record_object = etree.Element(self.category)
         record_object.set('url_name', self.url_name)
 
         # Special case for course pointers: