Merge pull request #373 from MITx/MITx/feature/bridger/fast_course_grading

Make templates render on non-xml modulestores

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

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,6 +304,7 @@ class XmlDescriptor(XModuleDescriptor):
             if attr not in self.metadata_to_strip:
                 xml_object.set(attr, val_for_xml(attr))
 
+        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)
@@ -303,6 +313,9 @@ class XmlDescriptor(XModuleDescriptor):
 
             # And return just a pointer with the category and filename.
             record_object = etree.Element(self.category)
+        else:
+            record_object = xml_object
+
         record_object.set('url_name', self.url_name)
 
         # Special case for course pointers: