From 53608922ba2cf74f27e5f05a23694943593704f4 Mon Sep 17 00:00:00 2001
From: Victor Shnayder <victor@mitx.mit.edu>
Date: Mon, 30 Jul 2012 11:36:43 -0400
Subject: [PATCH] Make the malformed descriptor import properly

* Also get rid of lazy loading of metadata and definition
---
 common/lib/xmodule/setup.py                   |   1 +
 .../lib/xmodule/xmodule/malformed_module.py   |  11 +-
 common/lib/xmodule/xmodule/x_module.py        |   1 +
 common/lib/xmodule/xmodule/xml_module.py      | 125 +++++++++---------
 4 files changed, 74 insertions(+), 64 deletions(-)

diff --git a/common/lib/xmodule/setup.py b/common/lib/xmodule/setup.py
index b495cd0aeea..09a6b7e7129 100644
--- a/common/lib/xmodule/setup.py
+++ b/common/lib/xmodule/setup.py
@@ -25,6 +25,7 @@ setup(
             "discuss = xmodule.backcompat_module:TranslateCustomTagDescriptor",
             "html = xmodule.html_module:HtmlDescriptor",
             "image = xmodule.backcompat_module:TranslateCustomTagDescriptor",
+            "malformed = xmodule.malformed_module:MalformedDescriptor",
             "problem = xmodule.capa_module:CapaDescriptor",
             "problemset = xmodule.vertical_module:VerticalDescriptor",
             "section = xmodule.backcompat_module:SemanticSectionDescriptor",
diff --git a/common/lib/xmodule/xmodule/malformed_module.py b/common/lib/xmodule/xmodule/malformed_module.py
index 803813dee8a..54032cf7d22 100644
--- a/common/lib/xmodule/xmodule/malformed_module.py
+++ b/common/lib/xmodule/xmodule/malformed_module.py
@@ -20,8 +20,15 @@ class MalformedDescriptor(EditingDescriptor):
         Does not try to parse the data--just stores it.
         '''
 
-        # TODO (vshnayder): how does one get back from this to a valid descriptor?
-        # try to parse and if successfull, send back to x_module?
+        #log.debug("processing '{0}'".format(xml_data))
+        try:
+            xml_obj = etree.fromstring(xml_data)
+            if xml_obj.tag == 'malformed':
+                xml_data = xml_obj.text
+            # TODO (vshnayder): how does one get back from this to a valid descriptor?
+            # For now, have to fix manually.
+        except etree.XMLSyntaxError:
+            pass
 
         definition = { 'data' : xml_data }
         # TODO (vshnayder): Do we need a valid slug here?  Just pick a random
diff --git a/common/lib/xmodule/xmodule/x_module.py b/common/lib/xmodule/xmodule/x_module.py
index c4c5110abc0..8803c37d788 100644
--- a/common/lib/xmodule/xmodule/x_module.py
+++ b/common/lib/xmodule/xmodule/x_module.py
@@ -464,6 +464,7 @@ class XModuleDescriptor(Plugin, HTMLSnippet):
             # Put import here to avoid circular import errors
             from xmodule.malformed_module import MalformedDescriptor
 
+            #system.error_handler("Error loading from xml.")
             descriptor = MalformedDescriptor.from_xml(xml_data, system, org, course)
 
         return descriptor
diff --git a/common/lib/xmodule/xmodule/xml_module.py b/common/lib/xmodule/xmodule/xml_module.py
index 6750906eb44..5786dbd227e 100644
--- a/common/lib/xmodule/xmodule/xml_module.py
+++ b/common/lib/xmodule/xmodule/xml_module.py
@@ -11,66 +11,65 @@ 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
-
-
-class LazyLoadingDict(MutableMapping):
-    """
-    A dictionary object that lazily loads its contents from a provided
-    function on reads (of members that haven't already been set).
-    """
-
-    def __init__(self, loader):
-        '''
-        On the first read from this dictionary, it will call loader() to
-        populate its contents.  loader() must return something dict-like. Any
-        elements set before the first read will be preserved.
-        '''
-        self._contents = {}
-        self._loaded = False
-        self._loader = loader
-        self._deleted = set()
-
-    def __getitem__(self, name):
-        if not (self._loaded or name in self._contents or name in self._deleted):
-            self.load()
-
-        return self._contents[name]
-
-    def __setitem__(self, name, value):
-        self._contents[name] = value
-        self._deleted.discard(name)
-
-    def __delitem__(self, name):
-        del self._contents[name]
-        self._deleted.add(name)
-
-    def __contains__(self, name):
-        self.load()
-        return name in self._contents
-
-    def __len__(self):
-        self.load()
-        return len(self._contents)
-
-    def __iter__(self):
-        self.load()
-        return iter(self._contents)
-
-    def __repr__(self):
-        self.load()
-        return repr(self._contents)
-
-    def load(self):
-        if self._loaded:
-            return
-
-        loaded_contents = self._loader()
-        loaded_contents.update(self._contents)
-        self._contents = loaded_contents
-        self._loaded = True
+# # 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
+
+# class LazyLoadingDict(MutableMapping):
+#     """
+#     A dictionary object that lazily loads its contents from a provided
+#     function on reads (of members that haven't already been set).
+#     """
+
+#     def __init__(self, loader):
+#         '''
+#         On the first read from this dictionary, it will call loader() to
+#         populate its contents.  loader() must return something dict-like. Any
+#         elements set before the first read will be preserved.
+#         '''
+#         self._contents = {}
+#         self._loaded = False
+#         self._loader = loader
+#         self._deleted = set()
+
+#     def __getitem__(self, name):
+#         if not (self._loaded or name in self._contents or name in self._deleted):
+#             self.load()
+
+#         return self._contents[name]
+
+#     def __setitem__(self, name, value):
+#         self._contents[name] = value
+#         self._deleted.discard(name)
+
+#     def __delitem__(self, name):
+#         del self._contents[name]
+#         self._deleted.add(name)
+
+#     def __contains__(self, name):
+#         self.load()
+#         return name in self._contents
+
+#     def __len__(self):
+#         self.load()
+#         return len(self._contents)
+
+#     def __iter__(self):
+#         self.load()
+#         return iter(self._contents)
+
+#     def __repr__(self):
+#         self.load()
+#         return repr(self._contents)
+
+#     def load(self):
+#         if self._loaded:
+#             return
+
+#         loaded_contents = self._loader()
+#         loaded_contents.update(self._contents)
+#         self._contents = loaded_contents
+#         self._loaded = True
 
 
 _AttrMapBase = namedtuple('_AttrMap', 'metadata_key to_metadata from_metadata')
@@ -230,11 +229,13 @@ class XmlDescriptor(XModuleDescriptor):
             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),
+            definition,
             location=location,
-            metadata=LazyLoadingDict(metadata_loader),
+            metadata=metadata,
         )
 
     @classmethod
-- 
GitLab