From b285f50d92f02b197f16c0ee448ae26436c73397 Mon Sep 17 00:00:00 2001 From: Victor Shnayder Date: Fri, 3 Aug 2012 15:28:48 -0400 Subject: [PATCH] Make unknown metadata persist accross import-export * +improve test. --- .../lib/xmodule/xmodule/tests/test_import.py | 40 ++++++++++++++----- common/lib/xmodule/xmodule/xml_module.py | 17 ++++---- 2 files changed, 39 insertions(+), 18 deletions(-) diff --git a/common/lib/xmodule/xmodule/tests/test_import.py b/common/lib/xmodule/xmodule/tests/test_import.py index 2599a74079..45a142b8fe 100644 --- a/common/lib/xmodule/xmodule/tests/test_import.py +++ b/common/lib/xmodule/xmodule/tests/test_import.py @@ -111,30 +111,50 @@ class ImportTestCase(unittest.TestCase): xml_out = etree.fromstring(xml_str_out) self.assertEqual(xml_out.tag, 'sequential') - def test_metadata_inherit(self): - """Make sure metadata inherits properly""" + def test_metadata_import_export(self): + """Two checks: + - unknown metadata is preserved across import-export + - inherited metadata doesn't leak to children. + """ system = self.get_system() v = "1 hour" - start_xml = ''' - - Two houses, ... - '''.format(grace=v) + start_xml = ''' + + + Two houses, ... + + '''.format(grace=v) descriptor = XModuleDescriptor.load_from_xml(start_xml, system, 'org', 'course') - print "Errors: {0}".format(system.errorlog.errors) print descriptor, descriptor.metadata self.assertEqual(descriptor.metadata['graceperiod'], v) + self.assertEqual(descriptor.metadata['unicorn'], 'purple') - # Check that the child inherits correctly + # Check that the child inherits graceperiod correctly child = descriptor.get_children()[0] self.assertEqual(child.metadata['graceperiod'], v) - # Now export and see if the chapter tag has a graceperiod attribute + # check that the child does _not_ inherit any unicorns + self.assertTrue('unicorn' not in child.metadata) + + # Now export and check things resource_fs = MemoryFS() exported_xml = descriptor.export_to_xml(resource_fs) print "Exported xml:", exported_xml - # hardcode path to child + + # Does the course still have unicorns? + # hardcoded path to course + with resource_fs.open('course/test1.xml') as f: + course_xml = etree.fromstring(f.read()) + + self.assertEqual(course_xml.attrib['unicorn'], 'purple') + + # did we successfully strip the url_name from the definition contents? + self.assertTrue('url_name' not in course_xml.attrib) + + # Does the chapter tag now have a graceperiod attribute? + # hardcoded path to child with resource_fs.open('chapter/ch.xml') as f: chapter_xml = etree.fromstring(f.read()) self.assertEqual(chapter_xml.tag, 'chapter') diff --git a/common/lib/xmodule/xmodule/xml_module.py b/common/lib/xmodule/xmodule/xml_module.py index fd48439e46..907fc302a3 100644 --- a/common/lib/xmodule/xmodule/xml_module.py +++ b/common/lib/xmodule/xmodule/xml_module.py @@ -47,6 +47,10 @@ class XmlDescriptor(XModuleDescriptor): # VS[compat] Remove once unused. 'name', 'slug') + # VS[compat] -- remove once everything is in the CMS + # We don't want url_name in the metadata--it's in the location, so avoid + # confusion and duplication. + metadata_to_strip = ('url_name', ) # A dictionary mapping xml attribute names AttrMaps that describe how # to import and export them @@ -166,12 +170,16 @@ class XmlDescriptor(XModuleDescriptor): Returns a dictionary {key: value}. """ metadata = {} - for attr in cls.metadata_attributes: + for attr in xml_object.attrib: val = xml_object.get(attr) if val is not None: # VS[compat]. Remove after all key translations done attr = cls._translate(attr) + if attr in cls.metadata_to_strip: + # don't load these + continue + attr_map = cls.xml_attribute_map.get(attr, AttrMap()) metadata[attr] = attr_map.from_xml(val) return metadata @@ -250,15 +258,8 @@ class XmlDescriptor(XModuleDescriptor): # Add the non-inherited metadata for attr in self.own_metadata: - if attr not in self.metadata_attributes: - log.warning("Unexpected metadata '{attr}' on element '{url}'." - " Not exporting.".format(attr=attr, - url=self.location.url())) - continue - xml_object.set(attr, val_for_xml(attr)) - # Write the actual contents to a file filepath = self.__class__._format_filepath(self.category, self.url_name) resource_fs.makedir(os.path.dirname(filepath), allow_recreate=True)