diff --git a/common/lib/xmodule/xmodule/modulestore/xml.py b/common/lib/xmodule/xmodule/modulestore/xml.py index 23b812baae..047e91e084 100644 --- a/common/lib/xmodule/xmodule/modulestore/xml.py +++ b/common/lib/xmodule/xmodule/modulestore/xml.py @@ -50,24 +50,25 @@ class ImportSystem(XMLParsingSystem, MakoDescriptorSystem): log.exception("Unable to parse xml: {xml}".format(xml=xml)) raise - # TODO (vshnayder): is the slug munging permanent, or also intended - # to be taken out? - if xml_data.get('slug') is None: + # VS[compat]. Take this out once course conversion is done + if xml_data.get('slug') is None and xml_data.get('url_name') is None: if xml_data.get('name'): slug = Location.clean(xml_data.get('name')) + elif xml_data.get('display_name'): + slug = Location.clean(xml_data.get('display_name')) else: self.unnamed_modules += 1 slug = '{tag}_{count}'.format(tag=xml_data.tag, count=self.unnamed_modules) - if slug in self.used_slugs: + while slug in self.used_slugs: self.unnamed_modules += 1 slug = '{slug}_{count}'.format(slug=slug, count=self.unnamed_modules) self.used_slugs.add(slug) # log.debug('-> slug=%s' % slug) - xml_data.set('slug', slug) + xml_data.set('url_name', slug) module = XModuleDescriptor.load_from_xml( etree.tostring(xml_data), self, org, diff --git a/common/lib/xmodule/xmodule/xml_module.py b/common/lib/xmodule/xmodule/xml_module.py index 2c3fa62f6e..8327a3ddec 100644 --- a/common/lib/xmodule/xmodule/xml_module.py +++ b/common/lib/xmodule/xmodule/xml_module.py @@ -9,7 +9,6 @@ import os log = logging.getLogger(__name__) - # TODO (cpennington): This was implemented in an attempt to improve performance, # but the actual improvement wasn't measured (and it was implemented late at night). # We should check if it hurts, and whether there's a better way of doing lazy loading @@ -76,8 +75,13 @@ _AttrMapBase = namedtuple('_AttrMap', 'metadata_key to_metadata from_metadata') class AttrMap(_AttrMapBase): """ - A class that specifies a metadata_key, a function to transform an xml - attribute to be placed in that key, and to transform that key value + A class that specifies a metadata_key, and two functions: + + to_metadata: convert value from the xml representation into + an internal python representation + + from_metadata: convert the internal python representation into + the value to store in the xml. """ def __new__(_cls, metadata_key, to_metadata=lambda x: x, @@ -96,17 +100,35 @@ class XmlDescriptor(XModuleDescriptor): # The attributes will be removed from the definition xml passed # to definition_from_xml, and from the xml returned by definition_to_xml metadata_attributes = ('format', 'graceperiod', 'showanswer', 'rerandomize', - 'start', 'due', 'graded', 'name', 'slug', 'hide_from_toc') + 'start', 'due', 'graded', 'display_name', 'url_name', 'hide_from_toc', + # VS[compat] Remove once unused. + 'name', 'slug') - # A dictionary mapping xml attribute names to functions of the value - # that return the metadata key and value + + # A dictionary mapping xml attribute names AttrMaps that describe how + # to import and export them xml_attribute_map = { + # type conversion: want True/False in python, "true"/"false" in xml 'graded': AttrMap('graded', lambda val: val == 'true', lambda val: str(val).lower()), - 'name': AttrMap('display_name'), } + + # VS[compat]. Backwards compatibility code that can go away after + # importing 2012 courses. + # A set of metadata key conversions that we want to make + metadata_translations = { + 'slug' : 'url_name', + 'name' : 'display_name', + } + + @classmethod + def _translate(cls, key): + 'VS[compat]' + return cls.metadata_translations.get(key, key) + + @classmethod def definition_from_xml(cls, xml_object, system): """ @@ -157,9 +179,11 @@ class XmlDescriptor(XModuleDescriptor): for attr in cls.metadata_attributes: val = xml_object.get(attr) if val is not None: + # VS[compat]. Remove after all key translations done + attr = cls._translate(attr) + attr_map = cls.xml_attribute_map.get(attr, AttrMap(attr)) metadata[attr_map.metadata_key] = attr_map.to_metadata(val) - return metadata def definition_loader(): @@ -182,7 +206,6 @@ class XmlDescriptor(XModuleDescriptor): filepath = candidate break - log.debug('filepath=%s, resources_fs=%s' % (filepath, system.resources_fs)) try: with system.resources_fs.open(filepath) as file: definition_xml = cls.file_to_xml(file) @@ -190,12 +213,14 @@ class XmlDescriptor(XModuleDescriptor): msg = 'Unable to load file contents at path %s' % filepath log.exception(msg) system.error_handler(msg) - # if error_handler didn't reraise, work around it. + # if error_handler didn't reraise, work around problem. return {'data': 'Error loading file contents at path %s' % filepath} cls.clean_metadata_from_xml(definition_xml) return cls.definition_from_xml(definition_xml, system) + # VS[compat] -- just have the url_name lookup once translation is done + slug = xml_object.get('url_name', xml_object.get('slug')) return cls( system, LazyLoadingDict(definition_loader), @@ -203,7 +228,7 @@ class XmlDescriptor(XModuleDescriptor): org, course, xml_object.tag, - xml_object.get('slug')], + slug], metadata=LazyLoadingDict(metadata_loader), ) @@ -242,12 +267,15 @@ class XmlDescriptor(XModuleDescriptor): resource_fs is a pyfilesystem object (from the fs package) """ + + # Get the definition xml_object = self.definition_to_xml(resource_fs) self.__class__.clean_metadata_from_xml(xml_object) + # Set the tag first, so it's right if writing to a file xml_object.tag = self.category - xml_object.set('slug', self.name) + # Write it to a file if necessary if self.split_to_file(xml_object): # Put this object in it's own file filepath = self.__class__._format_filepath(self.category, self.name) @@ -265,6 +293,8 @@ class XmlDescriptor(XModuleDescriptor): xml_object.set('filename', self.name) + # Add the metadata + xml_object.set('url_name', self.name) for attr in self.metadata_attributes: attr_map = self.xml_attribute_map.get(attr, AttrMap(attr)) metadata_key = attr_map.metadata_key @@ -276,6 +306,7 @@ class XmlDescriptor(XModuleDescriptor): val = attr_map.from_metadata(self.metadata[metadata_key]) xml_object.set(attr, val) + # Now we just have to make it beautiful return etree.tostring(xml_object, pretty_print=True) def definition_to_xml(self, resource_fs):