From a7d0e2e12236cb6f0ca2a86423c6f94bff872858 Mon Sep 17 00:00:00 2001
From: Calen Pennington <calen.pennington@gmail.com>
Date: Fri, 29 Jun 2012 13:29:04 -0400
Subject: [PATCH 01/38] Use a string key for default groups in abtests (and
 remove code that was expecting groups from django)

---
 common/lib/xmodule/abtest_module.py | 22 ++++++----------------
 1 file changed, 6 insertions(+), 16 deletions(-)

diff --git a/common/lib/xmodule/abtest_module.py b/common/lib/xmodule/abtest_module.py
index beaeb4ad1c..648082bf94 100644
--- a/common/lib/xmodule/abtest_module.py
+++ b/common/lib/xmodule/abtest_module.py
@@ -7,6 +7,8 @@ from xmodule.raw_module import RawDescriptor
 from xmodule.xml_module import XmlDescriptor
 from xmodule.exceptions import InvalidDefinitionError
 
+DEFAULT = "_DEFAULT_GROUP"
+
 
 def group_from_value(groups, v):
     ''' Given group: (('a',0.3),('b',0.4),('c',0.3)) And random value
@@ -39,7 +41,6 @@ class ABTestModule(XModule):
     def __init__(self, system, location, definition, instance_state=None, shared_state=None, **kwargs):
         XModule.__init__(self, system, location, definition, instance_state, shared_state, **kwargs)
 
-        target_groups = self.definition['data'].keys()
         if shared_state is None:
 
             self.group = group_from_value(
@@ -48,18 +49,7 @@ class ABTestModule(XModule):
             )
         else:
             shared_state = json.loads(shared_state)
-
-            # TODO (cpennington): Remove this once we aren't passing in
-            # groups from django groups
-            if 'groups' in shared_state:
-                self.group = None
-                target_names = [elem.get('name') for elem in target_groups]
-                for group in shared_state['groups']:
-                    if group in target_names:
-                        self.group = group
-                        break
-            else:
-                self.group = shared_state['group']
+            self.group = shared_state['group']
 
     def get_shared_state(self):
         print self.group
@@ -89,12 +79,12 @@ class ABTestDescriptor(RawDescriptor, XmlDescriptor):
             'data': {
                 'experiment': experiment,
                 'group_portions': [],
-                'group_content': {None: []},
+                'group_content': {DEFAULT: []},
             },
             'children': []}
         for group in xml_object:
             if group.tag == 'default':
-                name = None
+                name = DEFAULT
             else:
                 name = group.get('name')
                 definition['data']['group_portions'].append(
@@ -113,6 +103,6 @@ class ABTestDescriptor(RawDescriptor, XmlDescriptor):
         if default_portion < 0:
             raise InvalidDefinitionError("ABTest portions must add up to less than or equal to 1")
 
-        definition['data']['group_portions'].append((None, default_portion))
+        definition['data']['group_portions'].append((DEFAULT, default_portion))
 
         return definition

From 520fac1aa2d27b7f04e442b40b6e654f9e9f6a6a Mon Sep 17 00:00:00 2001
From: Calen Pennington <calen.pennington@gmail.com>
Date: Fri, 29 Jun 2012 13:40:06 -0400
Subject: [PATCH 02/38] Enforce an index over the location key in mongo

---
 common/lib/keystore/mongo.py | 1 +
 1 file changed, 1 insertion(+)

diff --git a/common/lib/keystore/mongo.py b/common/lib/keystore/mongo.py
index 20c4ffde1a..4c50b634ea 100644
--- a/common/lib/keystore/mongo.py
+++ b/common/lib/keystore/mongo.py
@@ -15,6 +15,7 @@ class MongoModuleStore(ModuleStore):
             host=host,
             port=port
         )[db][collection]
+        self.collection.ensure_index('location')
 
         # Force mongo to report errors, at the expense of performance
         self.collection.safe = True

From f859457037577833fd784eed430d25bd8cccd745 Mon Sep 17 00:00:00 2001
From: Calen Pennington <calen.pennington@gmail.com>
Date: Fri, 29 Jun 2012 13:58:07 -0400
Subject: [PATCH 03/38] Cache loaded plugins in memory

---
 common/lib/xmodule/x_module.py | 32 ++++++++++++++++++++------------
 1 file changed, 20 insertions(+), 12 deletions(-)

diff --git a/common/lib/xmodule/x_module.py b/common/lib/xmodule/x_module.py
index 191cda6b06..f628abe5f1 100644
--- a/common/lib/xmodule/x_module.py
+++ b/common/lib/xmodule/x_module.py
@@ -23,6 +23,9 @@ class Plugin(object):
 
         entry_point: The name of the entry point to load plugins from
     """
+
+    _plugin_cache = None
+
     @classmethod
     def load_class(cls, identifier, default=None):
         """
@@ -33,20 +36,25 @@ class Plugin(object):
         If default is not None, will return default if no entry_point matching identifier
         is found. Otherwise, will raise a ModuleMissingError
         """
-        identifier = identifier.lower()
-        classes = list(pkg_resources.iter_entry_points(cls.entry_point, name=identifier))
-        if len(classes) > 1:
-            log.warning("Found multiple classes for {entry_point} with identifier {id}: {classes}. Returning the first one.".format(
-                entry_point=cls.entry_point,
-                id=identifier,
-                classes=", ".join(class_.module_name for class_ in classes)))
+        if cls._plugin_cache is None:
+            cls._plugin_cache = {}
 
-        if len(classes) == 0:
-            if default is not None:
-                return default
-            raise ModuleMissingError(identifier)
+        if identifier not in cls._plugin_cache:
+            identifier = identifier.lower()
+            classes = list(pkg_resources.iter_entry_points(cls.entry_point, name=identifier))
+            if len(classes) > 1:
+                log.warning("Found multiple classes for {entry_point} with identifier {id}: {classes}. Returning the first one.".format(
+                    entry_point=cls.entry_point,
+                    id=identifier,
+                    classes=", ".join(class_.module_name for class_ in classes)))
 
-        return classes[0].load()
+            if len(classes) == 0:
+                if default is not None:
+                    return default
+                raise ModuleMissingError(identifier)
+
+            cls._plugin_cache[identifier] = classes[0].load()
+        return cls._plugin_cache[identifier]
 
     @classmethod
     def load_classes(cls):

From 51a790173f370d1240034e5ae033540e63178ea0 Mon Sep 17 00:00:00 2001
From: Calen Pennington <calen.pennington@gmail.com>
Date: Fri, 29 Jun 2012 16:03:17 -0400
Subject: [PATCH 04/38] Only set the default etree parser options in the module
 that is starting the xml parsing

---
 cms/djangoapps/contentstore/management/commands/import.py | 4 ----
 lms/djangoapps/courseware/views.py                        | 5 -----
 2 files changed, 9 deletions(-)

diff --git a/cms/djangoapps/contentstore/management/commands/import.py b/cms/djangoapps/contentstore/management/commands/import.py
index e24111dbb7..12806debb7 100644
--- a/cms/djangoapps/contentstore/management/commands/import.py
+++ b/cms/djangoapps/contentstore/management/commands/import.py
@@ -4,14 +4,10 @@
 
 from django.core.management.base import BaseCommand, CommandError
 from keystore.django import keystore
-from lxml import etree
 from keystore.xml import XMLModuleStore
 
 unnamed_modules = 0
 
-etree.set_default_parser(etree.XMLParser(dtd_validation=False, load_dtd=False,
-                                         remove_comments=True))
-
 
 class Command(BaseCommand):
     help = \
diff --git a/lms/djangoapps/courseware/views.py b/lms/djangoapps/courseware/views.py
index 48e9bcc795..52a86fdee4 100644
--- a/lms/djangoapps/courseware/views.py
+++ b/lms/djangoapps/courseware/views.py
@@ -12,8 +12,6 @@ from mitxmako.shortcuts import render_to_response, render_to_string
 from django_future.csrf import ensure_csrf_cookie
 from django.views.decorators.cache import cache_control
 
-from lxml import etree
-
 from module_render import toc_for_course, get_module, get_section
 from models import StudentModuleCache
 from student.models import UserProfile
@@ -26,9 +24,6 @@ from courseware import grades
 
 log = logging.getLogger("mitx.courseware")
 
-etree.set_default_parser(etree.XMLParser(dtd_validation=False, load_dtd=False,
-                                         remove_comments=True))
-
 template_imports = {'urllib': urllib}
 
 

From 7b89b1eb54ae66cbdb97254db364832a43ceafa0 Mon Sep 17 00:00:00 2001
From: Calen Pennington <calen.pennington@gmail.com>
Date: Fri, 29 Jun 2012 16:04:45 -0400
Subject: [PATCH 05/38] Add ability to update modulestore metadata for a module
 separately from data or children

---
 .../contentstore/management/commands/import.py |  1 +
 common/lib/keystore/__init__.py                | 12 +++++++++++-
 common/lib/keystore/mongo.py                   | 18 +++++++++++++++++-
 common/lib/keystore/xml.py                     | 10 ++++++++++
 4 files changed, 39 insertions(+), 2 deletions(-)

diff --git a/cms/djangoapps/contentstore/management/commands/import.py b/cms/djangoapps/contentstore/management/commands/import.py
index 12806debb7..bb9697d6a1 100644
--- a/cms/djangoapps/contentstore/management/commands/import.py
+++ b/cms/djangoapps/contentstore/management/commands/import.py
@@ -26,3 +26,4 @@ class Command(BaseCommand):
                 keystore().update_item(module.location, module.definition['data'])
             if 'children' in module.definition:
                 keystore().update_children(module.location, module.definition['children'])
+            keystore().update_metadata(module.url, module.metadata)
diff --git a/common/lib/keystore/__init__.py b/common/lib/keystore/__init__.py
index 0671e7e568..43ffa464ff 100644
--- a/common/lib/keystore/__init__.py
+++ b/common/lib/keystore/__init__.py
@@ -171,9 +171,19 @@ class ModuleStore(object):
     def update_children(self, location, children):
         """
         Set the children for the item specified by the location to
-        data
+        children
 
         location: Something that can be passed to Location
         children: A list of child item identifiers
         """
         raise NotImplementedError
+
+    def update_metadata(self, location, metadata):
+        """
+        Set the metadata for the item specified by the location to
+        metadata
+
+        location: Something that can be passed to Location
+        metadata: A nested dictionary of module metadata
+        """
+        raise NotImplementedError
diff --git a/common/lib/keystore/mongo.py b/common/lib/keystore/mongo.py
index 4c50b634ea..d92782600c 100644
--- a/common/lib/keystore/mongo.py
+++ b/common/lib/keystore/mongo.py
@@ -85,7 +85,7 @@ class MongoModuleStore(ModuleStore):
     def update_children(self, location, children):
         """
         Set the children for the item specified by the location to
-        data
+        children
 
         location: Something that can be passed to Location
         children: A list of child item identifiers
@@ -97,3 +97,19 @@ class MongoModuleStore(ModuleStore):
             {'location': Location(location).dict()},
             {'$set': {'definition.children': children}}
         )
+
+    def update_metadata(self, location, metadata):
+        """
+        Set the children for the item specified by the location to
+        metadata
+
+        location: Something that can be passed to Location
+        metadata: A nested dictionary of module metadata
+        """
+
+        # See http://www.mongodb.org/display/DOCS/Updating for
+        # atomic update syntax
+        self.collection.update(
+            {'location': Location(location).dict()},
+            {'$set': {'metadata': metadata}}
+        )
diff --git a/common/lib/keystore/xml.py b/common/lib/keystore/xml.py
index e7adb56ad6..d475077733 100644
--- a/common/lib/keystore/xml.py
+++ b/common/lib/keystore/xml.py
@@ -94,3 +94,13 @@ class XMLModuleStore(ModuleStore):
         children: A list of child item identifiers
         """
         raise NotImplementedError("XMLModuleStores are read-only")
+
+    def update_metadata(self, location, metadata):
+        """
+        Set the metadata for the item specified by the location to
+        metadata
+
+        location: Something that can be passed to Location
+        metadata: A nested dictionary of module metadata
+        """
+        raise NotImplementedError("XMLModuleStores are read-only")

From 8a64029b079c4ac98b62a1be47bff2e1f57c1b89 Mon Sep 17 00:00:00 2001
From: Calen Pennington <calen.pennington@gmail.com>
Date: Fri, 29 Jun 2012 16:05:03 -0400
Subject: [PATCH 06/38] Remove blank text nodes during xml parsing

---
 common/lib/keystore/xml.py | 2 +-
 1 file changed, 1 insertion(+), 1 deletion(-)

diff --git a/common/lib/keystore/xml.py b/common/lib/keystore/xml.py
index d475077733..0672e4a7ff 100644
--- a/common/lib/keystore/xml.py
+++ b/common/lib/keystore/xml.py
@@ -9,7 +9,7 @@ from . import ModuleStore, Location
 from .exceptions import ItemNotFoundError
 
 etree.set_default_parser(etree.XMLParser(dtd_validation=False, load_dtd=False,
-                                         remove_comments=True))
+                                         remove_comments=True, remove_blank_text=True))
 
 log = logging.getLogger(__name__)
 

From b7062ca5ca4f3f80e626479de2dffaa878a62dde Mon Sep 17 00:00:00 2001
From: Calen Pennington <calen.pennington@gmail.com>
Date: Fri, 29 Jun 2012 16:05:20 -0400
Subject: [PATCH 07/38] Only set the xml slug if it isn't already set

---
 common/lib/keystore/xml.py | 11 ++++++-----
 1 file changed, 6 insertions(+), 5 deletions(-)

diff --git a/common/lib/keystore/xml.py b/common/lib/keystore/xml.py
index 0672e4a7ff..b078474a00 100644
--- a/common/lib/keystore/xml.py
+++ b/common/lib/keystore/xml.py
@@ -40,11 +40,12 @@ class XMLModuleStore(ModuleStore):
                         except:
                             log.exception("Unable to parse xml: {xml}".format(xml=xml))
                             raise
-                        if xml_data.get('name'):
-                            xml_data.set('slug', Location.clean(xml_data.get('name')))
-                        else:
-                            self.unnamed_modules += 1
-                            xml_data.set('slug', '{tag}_{count}'.format(tag=xml_data.tag, count=self.unnamed_modules))
+                        if xml_data.get('slug') is None:
+                            if xml_data.get('name'):
+                                xml_data.set('slug', Location.clean(xml_data.get('name')))
+                            else:
+                                self.unnamed_modules += 1
+                                xml_data.set('slug', '{tag}_{count}'.format(tag=xml_data.tag, count=self.unnamed_modules))
 
                         module = XModuleDescriptor.load_from_xml(etree.tostring(xml_data), self, org, course, modulestore.default_class)
                         modulestore.modules[module.location] = module

From 5b8c3dc1e4663a308eb0166a6a1ace893382fe40 Mon Sep 17 00:00:00 2001
From: Calen Pennington <calen.pennington@gmail.com>
Date: Fri, 29 Jun 2012 16:06:23 -0400
Subject: [PATCH 08/38] Make html a RawDescriptor with a slightly different UI

---
 cms/templates/widgets/html-edit.html | 4 ++--
 common/lib/xmodule/html_module.py    | 9 ++-------
 2 files changed, 4 insertions(+), 9 deletions(-)

diff --git a/cms/templates/widgets/html-edit.html b/cms/templates/widgets/html-edit.html
index 666aa1de81..f0f63ea905 100644
--- a/cms/templates/widgets/html-edit.html
+++ b/cms/templates/widgets/html-edit.html
@@ -33,8 +33,8 @@
     </section>
   </section>
 
-  <textarea name="" class="edit-box" rows="8" cols="40">${module.definition['data']}</textarea>
-  <div class="preview">${module.definition['data']}</div>
+  <textarea name="" class="edit-box" rows="8" cols="40">${data}</textarea>
+  <div class="preview">${data}</div>
 
   <div class="actions wip">
     <a href="" class="save-update">Save &amp; Update</a>
diff --git a/common/lib/xmodule/html_module.py b/common/lib/xmodule/html_module.py
index 32963600cd..307b1309e4 100644
--- a/common/lib/xmodule/html_module.py
+++ b/common/lib/xmodule/html_module.py
@@ -2,8 +2,7 @@ import json
 import logging
 
 from xmodule.x_module import XModule
-from xmodule.mako_module import MakoModuleDescriptor
-from xmodule.xml_module import XmlDescriptor
+from xmodule.raw_module import RawDescriptor
 from lxml import etree
 from pkg_resources import resource_string
 
@@ -19,7 +18,7 @@ class HtmlModule(XModule):
         self.html = self.definition['data']['text']
 
 
-class HtmlDescriptor(MakoModuleDescriptor, XmlDescriptor):
+class HtmlDescriptor(RawDescriptor):
     """
     Module for putting raw html in a course
     """
@@ -28,7 +27,3 @@ class HtmlDescriptor(MakoModuleDescriptor, XmlDescriptor):
 
     js = {'coffee': [resource_string(__name__, 'js/module/html.coffee')]}
     js_module = 'HTML'
-
-    @classmethod
-    def definition_from_xml(cls, xml_object, system):
-        return {'data': {'text': etree.tostring(xml_object)}}

From ada152758dbc5a164b11dc77f1f14e7d92fc8b8e Mon Sep 17 00:00:00 2001
From: Calen Pennington <calen.pennington@gmail.com>
Date: Fri, 29 Jun 2012 16:07:06 -0400
Subject: [PATCH 09/38] Make abtest store group portions as a dictionary

---
 common/lib/xmodule/abtest_module.py | 13 +++++--------
 1 file changed, 5 insertions(+), 8 deletions(-)

diff --git a/common/lib/xmodule/abtest_module.py b/common/lib/xmodule/abtest_module.py
index 648082bf94..0571cda4da 100644
--- a/common/lib/xmodule/abtest_module.py
+++ b/common/lib/xmodule/abtest_module.py
@@ -44,7 +44,7 @@ class ABTestModule(XModule):
         if shared_state is None:
 
             self.group = group_from_value(
-                self.definition['data']['group_portions'],
+                self.definition['data']['group_portions'].items(),
                 random.uniform(0, 1)
             )
         else:
@@ -52,7 +52,6 @@ class ABTestModule(XModule):
             self.group = shared_state['group']
 
     def get_shared_state(self):
-        print self.group
         return json.dumps({'group': self.group})
 
     def displayable_items(self):
@@ -78,7 +77,7 @@ class ABTestDescriptor(RawDescriptor, XmlDescriptor):
         definition = {
             'data': {
                 'experiment': experiment,
-                'group_portions': [],
+                'group_portions': {},
                 'group_content': {DEFAULT: []},
             },
             'children': []}
@@ -87,9 +86,7 @@ class ABTestDescriptor(RawDescriptor, XmlDescriptor):
                 name = DEFAULT
             else:
                 name = group.get('name')
-                definition['data']['group_portions'].append(
-                    (name, float(group.get('portion', 0)))
-                )
+                definition['data']['group_portions'][name] = float(group.get('portion', 0))
 
             child_content_urls = [
                 system.process_xml(etree.tostring(child)).location.url()
@@ -99,10 +96,10 @@ class ABTestDescriptor(RawDescriptor, XmlDescriptor):
             definition['data']['group_content'][name] = child_content_urls
             definition['children'].extend(child_content_urls)
 
-        default_portion = 1 - sum(portion for (name, portion) in definition['data']['group_portions'])
+        default_portion = 1 - sum(portion for (name, portion) in definition['data']['group_portions'].items())
         if default_portion < 0:
             raise InvalidDefinitionError("ABTest portions must add up to less than or equal to 1")
 
-        definition['data']['group_portions'].append((DEFAULT, default_portion))
+        definition['data']['group_portions'][DEFAULT] = default_portion
 
         return definition

From c6d5eea841aab2a65de73b3ceb635245b803d518 Mon Sep 17 00:00:00 2001
From: Calen Pennington <calen.pennington@gmail.com>
Date: Fri, 29 Jun 2012 16:07:22 -0400
Subject: [PATCH 10/38] Fix typo

---
 common/lib/xmodule/x_module.py | 2 +-
 1 file changed, 1 insertion(+), 1 deletion(-)

diff --git a/common/lib/xmodule/x_module.py b/common/lib/xmodule/x_module.py
index f628abe5f1..91d1bce1f4 100644
--- a/common/lib/xmodule/x_module.py
+++ b/common/lib/xmodule/x_module.py
@@ -29,7 +29,7 @@ class Plugin(object):
     @classmethod
     def load_class(cls, identifier, default=None):
         """
-        Loads a single class intance specified by identifier. If identifier
+        Loads a single class instance specified by identifier. If identifier
         specifies more than a single class, then logs a warning and returns the first
         class identified.
 

From a94e4d2f1b43e69c466e26bfa0e73ff1e8e30bed Mon Sep 17 00:00:00 2001
From: Calen Pennington <calen.pennington@gmail.com>
Date: Fri, 29 Jun 2012 16:07:51 -0400
Subject: [PATCH 11/38] Rearrange x_module definition into sections

---
 common/lib/xmodule/x_module.py | 90 ++++++++++++++++++++--------------
 1 file changed, 53 insertions(+), 37 deletions(-)

diff --git a/common/lib/xmodule/x_module.py b/common/lib/xmodule/x_module.py
index 91d1bce1f4..a2019cd5bf 100644
--- a/common/lib/xmodule/x_module.py
+++ b/common/lib/xmodule/x_module.py
@@ -213,6 +213,7 @@ class XModuleDescriptor(Plugin):
     # A list of metadata that this module can inherit from its parent module
     inheritable_metadata = ('graded', 'due', 'graceperiod', 'showanswer', 'rerandomize')
 
+    # ============================= STRUCTURAL MANIPULATION ===========================
     def __init__(self,
                  system,
                  definition=None,
@@ -253,6 +254,43 @@ class XModuleDescriptor(Plugin):
 
         self._child_instances = None
 
+    def inherit_metadata(self, metadata):
+        """
+        Updates this module with metadata inherited from a containing module.
+        Only metadata specified in self.inheritable_metadata will
+        be inherited
+        """
+        # Set all inheritable metadata from kwargs that are
+        # in self.inheritable_metadata and aren't already set in metadata
+        for attr in self.inheritable_metadata:
+            if attr not in self.metadata and attr in metadata:
+                self.metadata[attr] = metadata[attr]
+
+    def get_children(self):
+        """Returns a list of XModuleDescriptor instances for the children of this module"""
+        if self._child_instances is None:
+            self._child_instances = []
+            for child_loc in self.definition.get('children', []):
+                child = self.system.load_item(child_loc)
+                child.inherit_metadata(self.metadata)
+                self._child_instances.append(child)
+
+        return self._child_instances
+
+    def xmodule_constructor(self, system):
+        """
+        Returns a constructor for an XModule. This constructor takes two arguments:
+        instance_state and shared_state, and returns a fully nstantiated XModule
+        """
+        return partial(
+            self.module_class,
+            system,
+            self.location,
+            self.definition,
+            metadata=self.metadata
+        )
+
+    # ================================= JSON PARSING ===================================
     @staticmethod
     def load_from_json(json_data, system, default_class=None):
         """
@@ -280,6 +318,7 @@ class XModuleDescriptor(Plugin):
         """
         return cls(system=system, **json_data)
 
+    # ================================= XML PARSING ====================================
     @staticmethod
     def load_from_xml(xml_data,
             system,
@@ -315,6 +354,20 @@ class XModuleDescriptor(Plugin):
         """
         raise NotImplementedError('Modules must implement from_xml to be parsable from xml')
 
+    def export_to_xml(self, resource_fs):
+        """
+        Returns an xml string representing this module, and all modules underneath it.
+        May also write required resources out to resource_fs
+
+        Assumes that modules have single parantage (that no module appears twice in the same course),
+        and that it is thus safe to nest modules as xml children as appropriate.
+
+        The returned XML should be able to be parsed back into an identical XModuleDescriptor
+        using the from_xml method with the same system, org, and course
+        """
+        raise NotImplementedError('Modules must implement export_to_xml to enable xml export')
+
+    # ================================== HTML INTERFACE DEFINITIONS ======================
     @classmethod
     def get_javascript(cls):
         """
@@ -334,49 +387,12 @@ class XModuleDescriptor(Plugin):
         """
         return self.js_module
 
-
-    def inherit_metadata(self, metadata):
-        """
-        Updates this module with metadata inherited from a containing module.
-        Only metadata specified in self.inheritable_metadata will
-        be inherited
-        """
-        # Set all inheritable metadata from kwargs that are
-        # in self.inheritable_metadata and aren't already set in metadata
-        for attr in self.inheritable_metadata:
-            if attr not in self.metadata and attr in metadata:
-                self.metadata[attr] = metadata[attr]
-
-    def get_children(self):
-        """Returns a list of XModuleDescriptor instances for the children of this module"""
-        if self._child_instances is None:
-            self._child_instances = []
-            for child_loc in self.definition.get('children', []):
-                child = self.system.load_item(child_loc)
-                child.inherit_metadata(self.metadata)
-                self._child_instances.append(child)
-
-        return self._child_instances
-
     def get_html(self):
         """
         Return the html used to edit this module
         """
         raise NotImplementedError("get_html() must be provided by specific modules")
 
-    def xmodule_constructor(self, system):
-        """
-        Returns a constructor for an XModule. This constructor takes two arguments:
-        instance_state and shared_state, and returns a fully nstantiated XModule
-        """
-        return partial(
-            self.module_class,
-            system,
-            self.location,
-            self.definition,
-            metadata=self.metadata
-        )
-
 
 class DescriptorSystem(object):
     def __init__(self, load_item, resources_fs):

From f375258b38bd99eaf8914e958e551f8c83ca581a Mon Sep 17 00:00:00 2001
From: Calen Pennington <calen.pennington@gmail.com>
Date: Fri, 29 Jun 2012 16:08:15 -0400
Subject: [PATCH 12/38] Add xml export infrastructure for all existing modules

---
 common/lib/xmodule/abtest_module.py | 18 ++++++++++++++++
 common/lib/xmodule/raw_module.py    |  3 +++
 common/lib/xmodule/seq_module.py    |  6 ++++++
 common/lib/xmodule/xml_module.py    | 33 +++++++++++++++++++++++++++++
 4 files changed, 60 insertions(+)

diff --git a/common/lib/xmodule/abtest_module.py b/common/lib/xmodule/abtest_module.py
index 0571cda4da..f6057171e5 100644
--- a/common/lib/xmodule/abtest_module.py
+++ b/common/lib/xmodule/abtest_module.py
@@ -103,3 +103,21 @@ class ABTestDescriptor(RawDescriptor, XmlDescriptor):
         definition['data']['group_portions'][DEFAULT] = default_portion
 
         return definition
+
+    def definition_to_xml(self, resource_fs):
+        xml_object = etree.Element('abtest')
+        xml_object.set('experiment', self.definition['data']['experiment'])
+        for name, group in self.definition['data']['group_content'].items():
+            if name == DEFAULT:
+                group_elem = etree.SubElement(xml_object, 'default')
+            else:
+                group_elem = etree.SubElement(xml_object, 'group', attrib={
+                    'portion': str(self.definition['data']['group_portions'][name]),
+                    'name': name,
+                })
+
+            for child_loc in group:
+                child = self.system.load_item(child_loc)
+                group_elem.append(etree.fromstring(child.export_to_xml(resource_fs)))
+
+        return xml_object
diff --git a/common/lib/xmodule/raw_module.py b/common/lib/xmodule/raw_module.py
index 43a92303ad..59f28ff4f0 100644
--- a/common/lib/xmodule/raw_module.py
+++ b/common/lib/xmodule/raw_module.py
@@ -21,3 +21,6 @@ class RawDescriptor(MakoModuleDescriptor, XmlDescriptor):
     @classmethod
     def definition_from_xml(cls, xml_object, system):
         return {'data': etree.tostring(xml_object)}
+
+    def definition_to_xml(self, resource_fs):
+        return etree.fromstring(self.definition['data'])
diff --git a/common/lib/xmodule/seq_module.py b/common/lib/xmodule/seq_module.py
index 6d493b96ad..9f00c3be87 100644
--- a/common/lib/xmodule/seq_module.py
+++ b/common/lib/xmodule/seq_module.py
@@ -108,3 +108,9 @@ class SequenceDescriptor(MakoModuleDescriptor, XmlDescriptor):
             system.process_xml(etree.tostring(child_module)).location.url()
             for child_module in xml_object
         ]}
+
+    def definition_to_xml(self, resource_fs):
+        xml_object = etree.Element('sequential')
+        for child in self.get_children():
+            xml_object.append(etree.fromstring(child.export_to_xml(resource_fs)))
+        return xml_object
diff --git a/common/lib/xmodule/xml_module.py b/common/lib/xmodule/xml_module.py
index b167e52e88..d6338aeb39 100644
--- a/common/lib/xmodule/xml_module.py
+++ b/common/lib/xmodule/xml_module.py
@@ -51,3 +51,36 @@ class XmlDescriptor(XModuleDescriptor):
                       xml_object.get('slug')],
             metadata=metadata,
         )
+
+    def export_to_xml(self, resource_fs):
+        """
+        Returns an xml string representing this module, and all modules underneath it.
+        May also write required resources out to resource_fs
+
+        Assumes that modules have single parantage (that no module appears twice in the same course),
+        and that it is thus safe to nest modules as xml children as appropriate.
+
+        The returned XML should be able to be parsed back into an identical XModuleDescriptor
+        using the from_xml method with the same system, org, and course
+        """
+        xml_object = self.definition_to_xml(resource_fs)
+        xml_object.set('slug', self.name)
+        xml_object.tag = self.type
+
+        for attr in ('format', 'graceperiod', 'showanswer', 'rerandomize', 'due'):
+            if attr in self.metadata:
+                xml_object.set(attr, self.metadata[attr])
+
+        if 'graded' in self.metadata:
+            xml_object.set('graded', str(self.metadata['graded']).lower())
+
+        if 'display_name' in self.metadata:
+            xml_object.set('name', self.metadata['display_name'])
+
+        return etree.tostring(xml_object, pretty_print=True)
+
+    def definition_to_xml(self, resource_fs):
+        """
+        Return a new etree Element object created from this modules definition.
+        """
+        raise NotImplementedError("%s does not implement definition_to_xml" % self.__class__.__name__)

From 1e8acbefac409c125ddc7b26ef5979727430a3b0 Mon Sep 17 00:00:00 2001
From: Calen Pennington <calen.pennington@gmail.com>
Date: Fri, 29 Jun 2012 16:08:35 -0400
Subject: [PATCH 13/38] Add a temporary url for testing xml export triggering

---
 cms/djangoapps/contentstore/views.py | 14 ++++++++++++++
 cms/urls.py                          |  1 +
 2 files changed, 15 insertions(+)

diff --git a/cms/djangoapps/contentstore/views.py b/cms/djangoapps/contentstore/views.py
index f7d5efe22a..d9036515a9 100644
--- a/cms/djangoapps/contentstore/views.py
+++ b/cms/djangoapps/contentstore/views.py
@@ -4,6 +4,7 @@ from django_future.csrf import ensure_csrf_cookie
 from django.http import HttpResponse
 import json
 
+from fs.osfs import OSFS
 
 @ensure_csrf_cookie
 def index(request):
@@ -32,3 +33,16 @@ def save_item(request):
     data = json.loads(request.POST['data'])
     keystore().update_item(item_id, data)
     return HttpResponse(json.dumps({}))
+
+
+def temp_force_export(request):
+    org = 'mit.edu'
+    course = '6002xs12'
+    name = '6.002_Spring_2012'
+    course = keystore().get_item(['i4x', org, course, 'course', name])
+    fs = OSFS('../data-export-test')
+    xml = course.export_to_xml(fs)
+    with fs.open('course.xml', 'w') as course_xml:
+        course_xml.write(xml)
+
+    return HttpResponse('Done')
diff --git a/cms/urls.py b/cms/urls.py
index d7314aafae..9d827c3fe3 100644
--- a/cms/urls.py
+++ b/cms/urls.py
@@ -8,4 +8,5 @@ urlpatterns = patterns('',
     url(r'^$', 'contentstore.views.index', name='index'),
     url(r'^edit_item$', 'contentstore.views.edit_item', name='edit_item'),
     url(r'^save_item$', 'contentstore.views.save_item', name='save_item'),
+    url(r'^temp_force_export$', 'contentstore.views.temp_force_export')
 )

From 2f95146b9be2cf31f5881b2feec1ac342bfbbcb9 Mon Sep 17 00:00:00 2001
From: Calen Pennington <calen.pennington@gmail.com>
Date: Fri, 29 Jun 2012 16:08:54 -0400
Subject: [PATCH 14/38] Just use the class name when complaining about
 definition_from_xml not being implemented

---
 common/lib/xmodule/xml_module.py | 2 +-
 1 file changed, 1 insertion(+), 1 deletion(-)

diff --git a/common/lib/xmodule/xml_module.py b/common/lib/xmodule/xml_module.py
index d6338aeb39..fa275c67d3 100644
--- a/common/lib/xmodule/xml_module.py
+++ b/common/lib/xmodule/xml_module.py
@@ -13,7 +13,7 @@ class XmlDescriptor(XModuleDescriptor):
         Return the definition to be passed to the newly created descriptor
         during from_xml
         """
-        raise NotImplementedError("%s does not implement definition_from_xml" % cls.__class__.__name__)
+        raise NotImplementedError("%s does not implement definition_from_xml" % cls.__name__)
 
     @classmethod
     def from_xml(cls, xml_data, system, org=None, course=None):

From b9dd30cd58b1657f45c65315385c897fa88761ac Mon Sep 17 00:00:00 2001
From: Calen Pennington <calen.pennington@gmail.com>
Date: Fri, 29 Jun 2012 16:26:04 -0400
Subject: [PATCH 15/38] Don't dump inherited metadata when exporting xml

---
 common/lib/xmodule/x_module.py   | 2 ++
 common/lib/xmodule/xml_module.py | 4 ++--
 2 files changed, 4 insertions(+), 2 deletions(-)

diff --git a/common/lib/xmodule/x_module.py b/common/lib/xmodule/x_module.py
index a2019cd5bf..8367120748 100644
--- a/common/lib/xmodule/x_module.py
+++ b/common/lib/xmodule/x_module.py
@@ -253,6 +253,7 @@ class XModuleDescriptor(Plugin):
         self.shared_state_key = kwargs.get('shared_state_key')
 
         self._child_instances = None
+        self._inherited_metadata = set()
 
     def inherit_metadata(self, metadata):
         """
@@ -264,6 +265,7 @@ class XModuleDescriptor(Plugin):
         # in self.inheritable_metadata and aren't already set in metadata
         for attr in self.inheritable_metadata:
             if attr not in self.metadata and attr in metadata:
+                self._inherited_metadata.add(attr)
                 self.metadata[attr] = metadata[attr]
 
     def get_children(self):
diff --git a/common/lib/xmodule/xml_module.py b/common/lib/xmodule/xml_module.py
index fa275c67d3..6639a77d3e 100644
--- a/common/lib/xmodule/xml_module.py
+++ b/common/lib/xmodule/xml_module.py
@@ -68,10 +68,10 @@ class XmlDescriptor(XModuleDescriptor):
         xml_object.tag = self.type
 
         for attr in ('format', 'graceperiod', 'showanswer', 'rerandomize', 'due'):
-            if attr in self.metadata:
+            if attr in self.metadata and attr not in self._inherited_metadata:
                 xml_object.set(attr, self.metadata[attr])
 
-        if 'graded' in self.metadata:
+        if 'graded' in self.metadata and 'graded' not in self._inherited_metadata:
             xml_object.set('graded', str(self.metadata['graded']).lower())
 
         if 'display_name' in self.metadata:

From d95be5aa24e85937253025b4ee47d326e2d1d778 Mon Sep 17 00:00:00 2001
From: Calen Pennington <calen.pennington@gmail.com>
Date: Fri, 29 Jun 2012 19:32:48 -0400
Subject: [PATCH 16/38] Fix html rendering after making it a RawDescriptor

---
 common/lib/xmodule/html_module.py | 2 +-
 1 file changed, 1 insertion(+), 1 deletion(-)

diff --git a/common/lib/xmodule/html_module.py b/common/lib/xmodule/html_module.py
index 307b1309e4..cf45ec3a18 100644
--- a/common/lib/xmodule/html_module.py
+++ b/common/lib/xmodule/html_module.py
@@ -15,7 +15,7 @@ class HtmlModule(XModule):
 
     def __init__(self, system, location, definition, instance_state=None, shared_state=None, **kwargs):
         XModule.__init__(self, system, location, definition, instance_state, shared_state, **kwargs)
-        self.html = self.definition['data']['text']
+        self.html = self.definition['data']
 
 
 class HtmlDescriptor(RawDescriptor):

From 1d4e1d55d5b172eb00e817a9bc9627528d178a56 Mon Sep 17 00:00:00 2001
From: Calen Pennington <calen.pennington@gmail.com>
Date: Fri, 29 Jun 2012 22:08:17 -0400
Subject: [PATCH 17/38] Enforce location uniqueness in xml keystore

---
 common/lib/keystore/xml.py | 12 ++++++++++--
 1 file changed, 10 insertions(+), 2 deletions(-)

diff --git a/common/lib/keystore/xml.py b/common/lib/keystore/xml.py
index b078474a00..baa54c4248 100644
--- a/common/lib/keystore/xml.py
+++ b/common/lib/keystore/xml.py
@@ -33,6 +33,7 @@ class XMLModuleStore(ModuleStore):
                     modulestore: the XMLModuleStore to store the loaded modules in
                     """
                     self.unnamed_modules = 0
+                    self.used_slugs = set()
 
                     def process_xml(xml):
                         try:
@@ -42,10 +43,17 @@ class XMLModuleStore(ModuleStore):
                             raise
                         if xml_data.get('slug') is None:
                             if xml_data.get('name'):
-                                xml_data.set('slug', Location.clean(xml_data.get('name')))
+                                slug = Location.clean(xml_data.get('name'))
                             else:
                                 self.unnamed_modules += 1
-                                xml_data.set('slug', '{tag}_{count}'.format(tag=xml_data.tag, count=self.unnamed_modules))
+                                slug = '{tag}_{count}'.format(tag=xml_data.tag, count=self.unnamed_modules)
+
+                            if slug in self.used_slugs:
+                                self.unnamed_modules += 1
+                                slug = '{slug}_{count}'.format(slug=slug, count=self.unnamed_modules)
+
+                            self.used_slugs.add(slug)
+                            xml_data.set('slug', slug)
 
                         module = XModuleDescriptor.load_from_xml(etree.tostring(xml_data), self, org, course, modulestore.default_class)
                         modulestore.modules[module.location] = module

From 79987666df6955d7b445c7201b8b77a380194b2b Mon Sep 17 00:00:00 2001
From: Calen Pennington <calen.pennington@gmail.com>
Date: Fri, 29 Jun 2012 22:26:11 -0400
Subject: [PATCH 18/38] Lazily load module definition and metadata as needed,
 rather than immediately

---
 .../management/commands/import.py             |  4 +-
 common/lib/keystore/xml.py                    | 13 +++-
 common/lib/xmodule/xml_module.py              | 74 ++++++++++++++++---
 lms/djangoapps/courseware/module_render.py    |  7 +-
 4 files changed, 82 insertions(+), 16 deletions(-)

diff --git a/cms/djangoapps/contentstore/management/commands/import.py b/cms/djangoapps/contentstore/management/commands/import.py
index bb9697d6a1..75f59a0ef6 100644
--- a/cms/djangoapps/contentstore/management/commands/import.py
+++ b/cms/djangoapps/contentstore/management/commands/import.py
@@ -19,11 +19,11 @@ class Command(BaseCommand):
 
         org, course, data_dir = args
 
-        module_store = XMLModuleStore(org, course, data_dir, 'xmodule.raw_module.RawDescriptor')
+        module_store = XMLModuleStore(org, course, data_dir, 'xmodule.raw_module.RawDescriptor', eager=True)
         for module in module_store.modules.itervalues():
             keystore().create_item(module.location)
             if 'data' in module.definition:
                 keystore().update_item(module.location, module.definition['data'])
             if 'children' in module.definition:
                 keystore().update_children(module.location, module.definition['children'])
-            keystore().update_metadata(module.url, module.metadata)
+            keystore().update_metadata(module.location, dict(module.metadata))
diff --git a/common/lib/keystore/xml.py b/common/lib/keystore/xml.py
index baa54c4248..52eaf0ce0f 100644
--- a/common/lib/keystore/xml.py
+++ b/common/lib/keystore/xml.py
@@ -18,7 +18,15 @@ class XMLModuleStore(ModuleStore):
     """
     An XML backed ModuleStore
     """
-    def __init__(self, org, course, data_dir, default_class=None):
+    def __init__(self, org, course, data_dir, default_class=None, eager=False):
+        """
+        Initialize an XMLModuleStore from data_dir
+
+        org, course: Strings to be used in module keys
+        data_dir: path to data directory containing course.xml
+        default_class: dot-separated string defining the default descriptor class to use if non is specified in entry_points
+        eager: If true, load the modules children immediately to force the entire course tree to be parsed
+        """
         self.data_dir = path(data_dir)
         self.modules = {}
 
@@ -57,6 +65,9 @@ class XMLModuleStore(ModuleStore):
 
                         module = XModuleDescriptor.load_from_xml(etree.tostring(xml_data), self, org, course, modulestore.default_class)
                         modulestore.modules[module.location] = module
+
+                        if eager:
+                            module.get_children()
                         return module
 
                     XMLParsingSystem.__init__(self, modulestore.get_item, OSFS(data_dir), process_xml)
diff --git a/common/lib/xmodule/xml_module.py b/common/lib/xmodule/xml_module.py
index 6639a77d3e..a224e4391d 100644
--- a/common/lib/xmodule/xml_module.py
+++ b/common/lib/xmodule/xml_module.py
@@ -1,7 +1,56 @@
+from collections import MutableMapping
 from xmodule.x_module import XModuleDescriptor
 from lxml import etree
 
 
+class LazyLoadingDict(MutableMapping):
+    """
+    A dictionary object that lazily loads it's contents from a provided
+    function on reads (of members that haven't already been set)
+    """
+
+    def __init__(self, loader):
+        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 load(self):
+        if self._loaded:
+            return
+
+        loaded_contents = self._loader()
+        loaded_contents.update(self._contents)
+        self._contents = loaded_contents
+        self._loaded = True
+
+
 class XmlDescriptor(XModuleDescriptor):
     """
     Mixin class for standardized parsing of from xml
@@ -29,27 +78,30 @@ class XmlDescriptor(XModuleDescriptor):
         """
         xml_object = etree.fromstring(xml_data)
 
-        metadata = {}
-        for attr in ('format', 'graceperiod', 'showanswer', 'rerandomize', 'due'):
-            from_xml = xml_object.get(attr)
-            if from_xml is not None:
-                metadata[attr] = from_xml
+        def metadata_loader():
+            metadata = {}
+            for attr in ('format', 'graceperiod', 'showanswer', 'rerandomize', 'due'):
+                from_xml = xml_object.get(attr)
+                if from_xml is not None:
+                    metadata[attr] = from_xml
 
-        if xml_object.get('graded') is not None:
-            metadata['graded'] = xml_object.get('graded') == 'true'
+            if xml_object.get('graded') is not None:
+                metadata['graded'] = xml_object.get('graded') == 'true'
 
-        if xml_object.get('name') is not None:
-            metadata['display_name'] = xml_object.get('name')
+            if xml_object.get('name') is not None:
+                metadata['display_name'] = xml_object.get('name')
+
+            return metadata
 
         return cls(
             system,
-            cls.definition_from_xml(xml_object, system),
+            LazyLoadingDict(lambda: cls.definition_from_xml(xml_object, system)),
             location=['i4x',
                       org,
                       course,
                       xml_object.tag,
                       xml_object.get('slug')],
-            metadata=metadata,
+            metadata=LazyLoadingDict(metadata_loader),
         )
 
     def export_to_xml(self, resource_fs):
diff --git a/lms/djangoapps/courseware/module_render.py b/lms/djangoapps/courseware/module_render.py
index 5119cc2910..4e5ee62e63 100644
--- a/lms/djangoapps/courseware/module_render.py
+++ b/lms/djangoapps/courseware/module_render.py
@@ -273,8 +273,11 @@ def add_histogram(module):
         module_id = module.id
         histogram = grade_histogram(module_id)
         render_histogram = len(histogram) > 0
-        staff_context = {'definition': json.dumps(module.definition, indent=4),
-                         'metadata': json.dumps(module.metadata, indent=4),
+
+        # Cast module.definition and module.metadata to dicts so that json can dump them
+        # even though they are lazily loaded
+        staff_context = {'definition': json.dumps(dict(module.definition), indent=4),
+                         'metadata': json.dumps(dict(module.metadata), indent=4),
                          'element_id': module.location.html_id(),
                          'histogram': json.dumps(histogram),
                          'render_histogram': render_histogram,

From 9c715b60a60801c60dc270697b4b009491a4f006 Mon Sep 17 00:00:00 2001
From: Calen Pennington <calen.pennington@gmail.com>
Date: Fri, 29 Jun 2012 22:42:33 -0400
Subject: [PATCH 19/38] Fix broken element ids for modules with .s in their
 names

---
 common/lib/keystore/__init__.py | 2 +-
 1 file changed, 1 insertion(+), 1 deletion(-)

diff --git a/common/lib/keystore/__init__.py b/common/lib/keystore/__init__.py
index 43ffa464ff..13ff322e83 100644
--- a/common/lib/keystore/__init__.py
+++ b/common/lib/keystore/__init__.py
@@ -119,7 +119,7 @@ class Location(_LocationBase):
         """
         Return a string with a version of the location that is safe for use in html id attributes
         """
-        return "-".join(str(v) for v in self if v is not None)
+        return "-".join(str(v) for v in self.list() if v is not None).replace('.', '_')
 
     def dict(self):
         return self.__dict__

From bacce3e65625b733cdcca3960e4241d481240730 Mon Sep 17 00:00:00 2001
From: Calen Pennington <calen.pennington@gmail.com>
Date: Fri, 29 Jun 2012 23:29:36 -0400
Subject: [PATCH 20/38] Load module contents from the file specified by the
 filename attribute

---
 common/lib/xmodule/html_module.py |  5 +++++
 common/lib/xmodule/xml_module.py  | 25 ++++++++++++++++++++++++-
 2 files changed, 29 insertions(+), 1 deletion(-)

diff --git a/common/lib/xmodule/html_module.py b/common/lib/xmodule/html_module.py
index cf45ec3a18..aac97b5fd3 100644
--- a/common/lib/xmodule/html_module.py
+++ b/common/lib/xmodule/html_module.py
@@ -24,6 +24,11 @@ class HtmlDescriptor(RawDescriptor):
     """
     mako_template = "widgets/html-edit.html"
     module_class = HtmlModule
+    filename_extension = "html"
 
     js = {'coffee': [resource_string(__name__, 'js/module/html.coffee')]}
     js_module = 'HTML'
+
+    @classmethod
+    def definition_from_file(cls, file, system):
+        return {'data': file.read()}
diff --git a/common/lib/xmodule/xml_module.py b/common/lib/xmodule/xml_module.py
index a224e4391d..79b90c2003 100644
--- a/common/lib/xmodule/xml_module.py
+++ b/common/lib/xmodule/xml_module.py
@@ -56,14 +56,29 @@ class XmlDescriptor(XModuleDescriptor):
     Mixin class for standardized parsing of from xml
     """
 
+    # Extension to append to filename paths
+    filename_extension = 'xml'
+
     @classmethod
     def definition_from_xml(cls, xml_object, system):
         """
         Return the definition to be passed to the newly created descriptor
         during from_xml
+
+        xml_object: An etree Element
         """
         raise NotImplementedError("%s does not implement definition_from_xml" % cls.__name__)
 
+    @classmethod
+    def definition_from_file(cls, file, system):
+        """
+        Return the definition to be passed to the newly created descriptor
+        during from_xml
+
+        file: File pointer
+        """
+        return cls.definition_from_xml(etree.parse(file), system)
+
     @classmethod
     def from_xml(cls, xml_data, system, org=None, course=None):
         """
@@ -93,9 +108,17 @@ class XmlDescriptor(XModuleDescriptor):
 
             return metadata
 
+        def definition_loader():
+            filename = xml_object.get('filename')
+            if filename is None:
+                return cls.definition_from_xml(xml_object, system)
+            else:
+                filepath = '{type}/{name}.{ext}'.format(type=xml_object.tag, name=filename, ext=cls.filename_extension)
+                return cls.definition_from_file(system.resources_fs.open(filepath), system)
+
         return cls(
             system,
-            LazyLoadingDict(lambda: cls.definition_from_xml(xml_object, system)),
+            LazyLoadingDict(definition_loader),
             location=['i4x',
                       org,
                       course,

From 552c199795bd060eb9907c9c5833b7aebe8c95fe Mon Sep 17 00:00:00 2001
From: Calen Pennington <calen.pennington@gmail.com>
Date: Sat, 30 Jun 2012 00:20:54 -0400
Subject: [PATCH 21/38] Export large xml as separate files. Note: inherited
 metadata is creeping into child nodes

---
 common/lib/xmodule/html_module.py |  6 ------
 common/lib/xmodule/xml_module.py  | 33 ++++++++++++++++++++-----------
 2 files changed, 21 insertions(+), 18 deletions(-)

diff --git a/common/lib/xmodule/html_module.py b/common/lib/xmodule/html_module.py
index aac97b5fd3..08fe4bbecc 100644
--- a/common/lib/xmodule/html_module.py
+++ b/common/lib/xmodule/html_module.py
@@ -1,9 +1,7 @@
-import json
 import logging
 
 from xmodule.x_module import XModule
 from xmodule.raw_module import RawDescriptor
-from lxml import etree
 from pkg_resources import resource_string
 
 log = logging.getLogger("mitx.courseware")
@@ -28,7 +26,3 @@ class HtmlDescriptor(RawDescriptor):
 
     js = {'coffee': [resource_string(__name__, 'js/module/html.coffee')]}
     js_module = 'HTML'
-
-    @classmethod
-    def definition_from_file(cls, file, system):
-        return {'data': file.read()}
diff --git a/common/lib/xmodule/xml_module.py b/common/lib/xmodule/xml_module.py
index 79b90c2003..6daf2bca36 100644
--- a/common/lib/xmodule/xml_module.py
+++ b/common/lib/xmodule/xml_module.py
@@ -69,16 +69,6 @@ class XmlDescriptor(XModuleDescriptor):
         """
         raise NotImplementedError("%s does not implement definition_from_xml" % cls.__name__)
 
-    @classmethod
-    def definition_from_file(cls, file, system):
-        """
-        Return the definition to be passed to the newly created descriptor
-        during from_xml
-
-        file: File pointer
-        """
-        return cls.definition_from_xml(etree.parse(file), system)
-
     @classmethod
     def from_xml(cls, xml_data, system, org=None, course=None):
         """
@@ -113,8 +103,9 @@ class XmlDescriptor(XModuleDescriptor):
             if filename is None:
                 return cls.definition_from_xml(xml_object, system)
             else:
-                filepath = '{type}/{name}.{ext}'.format(type=xml_object.tag, name=filename, ext=cls.filename_extension)
-                return cls.definition_from_file(system.resources_fs.open(filepath), system)
+                filepath = cls._format_filepath(xml_object.tag, filename)
+                with system.resources_fs.open(filepath) as file:
+                    return cls.definition_from_xml(etree.parse(file).getroot(), system)
 
         return cls(
             system,
@@ -127,6 +118,10 @@ class XmlDescriptor(XModuleDescriptor):
             metadata=LazyLoadingDict(metadata_loader),
         )
 
+    @classmethod
+    def _format_filepath(cls, type, name):
+        return '{type}/{name}.{ext}'.format(type=type, name=name, ext=cls.filename_extension)
+
     def export_to_xml(self, resource_fs):
         """
         Returns an xml string representing this module, and all modules underneath it.
@@ -139,6 +134,20 @@ class XmlDescriptor(XModuleDescriptor):
         using the from_xml method with the same system, org, and course
         """
         xml_object = self.definition_to_xml(resource_fs)
+
+        # Put content in a separate file if it's large (has more than 5 descendent tags)
+        if len(list(xml_object.iter())) > 5:
+
+            filepath = self.__class__._format_filepath(self.type, self.name)
+            resource_fs.makedir(self.type, allow_recreate=True)
+            with resource_fs.open(filepath, 'w') as file:
+                file.write(etree.tostring(xml_object, pretty_print=True))
+
+            for child in xml_object:
+                xml_object.remove(child)
+
+            xml_object.set('filename', self.name)
+
         xml_object.set('slug', self.name)
         xml_object.tag = self.type
 

From 8cf848b191545f4c509a76333b0c43bb103d57cc Mon Sep 17 00:00:00 2001
From: Calen Pennington <calen.pennington@gmail.com>
Date: Sat, 30 Jun 2012 09:11:40 -0400
Subject: [PATCH 22/38] Handle the filename for capa_module in the xml_module
 code, rather than specially in capa_module

---
 common/lib/capa/capa_problem.py   | 22 +++++++++-------------
 common/lib/xmodule/capa_module.py | 17 +++--------------
 2 files changed, 12 insertions(+), 27 deletions(-)

diff --git a/common/lib/capa/capa_problem.py b/common/lib/capa/capa_problem.py
index a06ac1d7b6..2e3620021b 100644
--- a/common/lib/capa/capa_problem.py
+++ b/common/lib/capa/capa_problem.py
@@ -68,14 +68,13 @@ class LoncapaProblem(object):
     Main class for capa Problems.
     '''
 
-    def __init__(self, fileobject, id, state=None, seed=None, system=None):
+    def __init__(self, problem_text, id, state=None, seed=None, system=None):
         '''
-        Initializes capa Problem.  The problem itself is defined by the XML file
-        pointed to by fileobject.
+        Initializes capa Problem.
 
         Arguments:
 
-         - filesobject  : an OSFS instance: see fs.osfs
+         - problem_text : xml defining the problem
          - id           : string used as the identifier for this problem; often a filename (no spaces)
          - state        : student state (represented as a dict)
          - seed         : random number generator seed (int)
@@ -103,14 +102,11 @@ class LoncapaProblem(object):
         if not self.seed:
             self.seed = struct.unpack('i', os.urandom(4))[0]
 
-        self.fileobject = fileobject    	# save problem file object, so we can use for debugging information later
-        if getattr(system, 'DEBUG', False):	# get the problem XML string from the problem file
-            log.info("[courseware.capa.capa_problem.lcp.init]  fileobject = %s" % fileobject)
-        file_text = fileobject.read()
-        file_text = re.sub("startouttext\s*/", "text", file_text)   # Convert startouttext and endouttext to proper <text></text>
-        file_text = re.sub("endouttext\s*/", "/text", file_text)
+        problem_text = re.sub("startouttext\s*/", "text", problem_text)   # Convert startouttext and endouttext to proper <text></text>
+        problem_text = re.sub("endouttext\s*/", "/text", problem_text)
+        self.problem_text = problem_text
 
-        self.tree = etree.XML(file_text)	# parse problem XML file into an element tree
+        self.tree = etree.XML(problem_text)  # parse problem XML file into an element tree
         self._process_includes()		# handle any <include file="foo"> tags
 
         # construct script processor context (eg for customresponse problems)
@@ -130,7 +126,7 @@ class LoncapaProblem(object):
         self.done = False
 
     def __unicode__(self):
-        return u"LoncapaProblem ({0})".format(self.fileobject)
+        return u"LoncapaProblem ({0})".format(self.problem_text)
 
     def get_state(self):
         ''' Stored per-user session data neeeded to:
@@ -272,7 +268,7 @@ class LoncapaProblem(object):
                 parent = inc.getparent()			# insert  new XML into tree in place of inlcude
                 parent.insert(parent.index(inc),incxml)
                 parent.remove(inc)
-                log.debug('Included %s into %s' % (file,self.fileobject))
+                log.debug('Included %s into %s' % (file, self.id))
 
     def _extract_context(self, tree, seed=struct.unpack('i', os.urandom(4))[0]):  # private
         '''
diff --git a/common/lib/xmodule/capa_module.py b/common/lib/xmodule/capa_module.py
index 6a95789417..57c5fa88ce 100644
--- a/common/lib/xmodule/capa_module.py
+++ b/common/lib/xmodule/capa_module.py
@@ -117,8 +117,6 @@ class CapaModule(XModule):
         if instance_state != None and 'attempts' in instance_state:
             self.attempts = instance_state['attempts']
 
-        # TODO: Should be: self.filename=only_one(dom2.xpath('/problem/@filename'))
-        self.filename = "problems/" + only_one(dom2.xpath('/problem/@filename')) + ".xml"
         self.name = only_one(dom2.xpath('/problem/@name'))
 
         weight_string = only_one(dom2.xpath('/problem/@weight'))
@@ -133,20 +131,11 @@ class CapaModule(XModule):
             seed = system.id
         else:
             seed = None
+
         try:
-            fp = self.system.filestore.open(self.filename)
+            self.lcp = LoncapaProblem(self.definition['data'], self.location.html_id(), instance_state, seed=seed, system=self.system)
         except Exception:
-            log.exception('cannot open file %s' % self.filename)
-            if self.system.DEBUG:
-                # create a dummy problem instead of failing
-                fp = StringIO.StringIO('<problem><text><font color="red" size="+2">Problem file %s is missing</font></text></problem>' % self.filename)
-                fp.name = "StringIO"
-            else:
-                raise
-        try:
-            self.lcp = LoncapaProblem(fp, self.location.html_id(), instance_state, seed=seed, system=self.system)
-        except Exception:
-            msg = 'cannot create LoncapaProblem %s' % self.filename
+            msg = 'cannot create LoncapaProblem %s' % self.url
             log.exception(msg)
             if self.system.DEBUG:
                 msg = '<p>%s</p>' % msg.replace('<', '&lt;')

From 6612beab463874aa03f10fdc27a9b14373438378 Mon Sep 17 00:00:00 2001
From: Calen Pennington <calen.pennington@gmail.com>
Date: Mon, 2 Jul 2012 11:07:17 -0400
Subject: [PATCH 23/38] Acknowledge the fact that right now keystore is really
 just a module store. If we need a keystore that returns other objects, we can
 reexctract the base class into it's own module again

---
 .../management/commands/import.py             | 14 +++++-----
 cms/djangoapps/contentstore/views.py          | 10 +++----
 cms/envs/dev.py                               |  4 +--
 cms/envs/test.py                              |  2 +-
 common/lib/keystore/django.py                 | 26 -------------------
 .../modulestore}/__init__.py                  |  4 +--
 common/lib/xmodule/modulestore/django.py      | 26 +++++++++++++++++++
 .../modulestore}/exceptions.py                |  0
 .../modulestore}/mongo.py                     |  4 +--
 .../modulestore}/tests/test_location.py       |  4 +--
 .../{keystore => xmodule/modulestore}/xml.py  |  4 +--
 .../xmodule/{tests.py => tests/__init__.py}   |  0
 common/lib/xmodule/x_module.py                |  4 +--
 .../management/commands/check_course.py       |  4 +--
 lms/djangoapps/courseware/module_render.py    | 10 +++----
 lms/djangoapps/courseware/views.py            |  6 ++---
 lms/envs/common.py                            |  4 +--
 17 files changed, 63 insertions(+), 63 deletions(-)
 delete mode 100644 common/lib/keystore/django.py
 rename common/lib/{keystore => xmodule/modulestore}/__init__.py (97%)
 create mode 100644 common/lib/xmodule/modulestore/django.py
 rename common/lib/{keystore => xmodule/modulestore}/exceptions.py (100%)
 rename common/lib/{keystore => xmodule/modulestore}/mongo.py (95%)
 rename common/lib/{keystore => xmodule/modulestore}/tests/test_location.py (95%)
 rename common/lib/{keystore => xmodule/modulestore}/xml.py (96%)
 rename common/lib/xmodule/{tests.py => tests/__init__.py} (100%)

diff --git a/cms/djangoapps/contentstore/management/commands/import.py b/cms/djangoapps/contentstore/management/commands/import.py
index 75f59a0ef6..f97ac10d41 100644
--- a/cms/djangoapps/contentstore/management/commands/import.py
+++ b/cms/djangoapps/contentstore/management/commands/import.py
@@ -3,15 +3,15 @@
 ###
 
 from django.core.management.base import BaseCommand, CommandError
-from keystore.django import keystore
-from keystore.xml import XMLModuleStore
+from xmodule.modulestore.django import modulestore
+from xmodule.modulestore.xml import XMLModuleStore
 
 unnamed_modules = 0
 
 
 class Command(BaseCommand):
     help = \
-'''Import the specified data directory into the default keystore'''
+'''Import the specified data directory into the default ModuleStore'''
 
     def handle(self, *args, **options):
         if len(args) != 3:
@@ -21,9 +21,9 @@ class Command(BaseCommand):
 
         module_store = XMLModuleStore(org, course, data_dir, 'xmodule.raw_module.RawDescriptor', eager=True)
         for module in module_store.modules.itervalues():
-            keystore().create_item(module.location)
+            modulestore().create_item(module.location)
             if 'data' in module.definition:
-                keystore().update_item(module.location, module.definition['data'])
+                modulestore().update_item(module.location, module.definition['data'])
             if 'children' in module.definition:
-                keystore().update_children(module.location, module.definition['children'])
-            keystore().update_metadata(module.location, dict(module.metadata))
+                modulestore().update_children(module.location, module.definition['children'])
+            modulestore().update_metadata(module.location, dict(module.metadata))
diff --git a/cms/djangoapps/contentstore/views.py b/cms/djangoapps/contentstore/views.py
index d9036515a9..76a904a403 100644
--- a/cms/djangoapps/contentstore/views.py
+++ b/cms/djangoapps/contentstore/views.py
@@ -1,5 +1,5 @@
 from mitxmako.shortcuts import render_to_response
-from keystore.django import keystore
+from xmodule.modulestore.django import modulestore
 from django_future.csrf import ensure_csrf_cookie
 from django.http import HttpResponse
 import json
@@ -12,14 +12,14 @@ def index(request):
     org = 'mit.edu'
     course = '6002xs12'
     name = '6.002_Spring_2012'
-    course = keystore().get_item(['i4x', org, course, 'course', name])
+    course = modulestore().get_item(['i4x', org, course, 'course', name])
     weeks = course.get_children()
     return render_to_response('index.html', {'weeks': weeks})
 
 
 def edit_item(request):
     item_id = request.GET['id']
-    item = keystore().get_item(item_id)
+    item = modulestore().get_item(item_id)
     return render_to_response('unit.html', {
         'contents': item.get_html(),
         'js_module': item.js_module_name(),
@@ -31,7 +31,7 @@ def edit_item(request):
 def save_item(request):
     item_id = request.POST['id']
     data = json.loads(request.POST['data'])
-    keystore().update_item(item_id, data)
+    modulestore().update_item(item_id, data)
     return HttpResponse(json.dumps({}))
 
 
@@ -39,7 +39,7 @@ def temp_force_export(request):
     org = 'mit.edu'
     course = '6002xs12'
     name = '6.002_Spring_2012'
-    course = keystore().get_item(['i4x', org, course, 'course', name])
+    course = modulestore().get_item(['i4x', org, course, 'course', name])
     fs = OSFS('../data-export-test')
     xml = course.export_to_xml(fs)
     with fs.open('course.xml', 'w') as course_xml:
diff --git a/cms/envs/dev.py b/cms/envs/dev.py
index ce775d962a..5bc5cfebc5 100644
--- a/cms/envs/dev.py
+++ b/cms/envs/dev.py
@@ -6,9 +6,9 @@ from .common import *
 DEBUG = True
 TEMPLATE_DEBUG = DEBUG
 
-KEYSTORE = {
+MODULESTORE = {
     'default': {
-        'ENGINE': 'keystore.mongo.MongoModuleStore',
+        'ENGINE': 'xmodule.modulestore.mongo.MongoModuleStore',
         'OPTIONS': {
             'default_class': 'xmodule.raw_module.RawDescriptor',
             'host': 'localhost',
diff --git a/cms/envs/test.py b/cms/envs/test.py
index 1a20d9e6f8..032de92953 100644
--- a/cms/envs/test.py
+++ b/cms/envs/test.py
@@ -17,7 +17,7 @@ for app in os.listdir(PROJECT_ROOT / 'djangoapps'):
     NOSE_ARGS += ['--cover-package', app]
 TEST_RUNNER = 'django_nose.NoseTestSuiteRunner'
 
-KEYSTORE = {
+MODULESTORE = {
     'host': 'localhost',
     'db': 'mongo_base',
     'collection': 'key_store',
diff --git a/common/lib/keystore/django.py b/common/lib/keystore/django.py
deleted file mode 100644
index 89aa9d07b0..0000000000
--- a/common/lib/keystore/django.py
+++ /dev/null
@@ -1,26 +0,0 @@
-"""
-Module that provides a connection to the keystore specified in the django settings.
-
-Passes settings.KEYSTORE as kwargs to MongoModuleStore
-"""
-
-from __future__ import absolute_import
-
-from importlib import import_module
-
-from django.conf import settings
-
-_KEYSTORES = {}
-
-
-def keystore(name='default'):
-    global _KEYSTORES
-
-    if name not in _KEYSTORES:
-        class_path = settings.KEYSTORE[name]['ENGINE']
-        module_path, _, class_name = class_path.rpartition('.')
-        class_ = getattr(import_module(module_path), class_name)
-        _KEYSTORES[name] = class_(
-            **settings.KEYSTORE[name]['OPTIONS'])
-
-    return _KEYSTORES[name]
diff --git a/common/lib/keystore/__init__.py b/common/lib/xmodule/modulestore/__init__.py
similarity index 97%
rename from common/lib/keystore/__init__.py
rename to common/lib/xmodule/modulestore/__init__.py
index 13ff322e83..00b3b13bb0 100644
--- a/common/lib/keystore/__init__.py
+++ b/common/lib/xmodule/modulestore/__init__.py
@@ -145,8 +145,8 @@ class ModuleStore(object):
         recent revision
 
         If any segment of the location is None except revision, raises
-            keystore.exceptions.InsufficientSpecificationError
-        If no object is found at that location, raises keystore.exceptions.ItemNotFoundError
+            xmodule.modulestore.exceptions.InsufficientSpecificationError
+        If no object is found at that location, raises xmodule.modulestore.exceptions.ItemNotFoundError
 
         location: Something that can be passed to Location
         default_class: An XModuleDescriptor subclass to use if no plugin matching the
diff --git a/common/lib/xmodule/modulestore/django.py b/common/lib/xmodule/modulestore/django.py
new file mode 100644
index 0000000000..546aaf30c8
--- /dev/null
+++ b/common/lib/xmodule/modulestore/django.py
@@ -0,0 +1,26 @@
+"""
+Module that provides a connection to the ModuleStore specified in the django settings.
+
+Passes settings.MODULESTORE as kwargs to MongoModuleStore
+"""
+
+from __future__ import absolute_import
+
+from importlib import import_module
+
+from django.conf import settings
+
+_MODULESTORES = {}
+
+
+def modulestore(name='default'):
+    global _MODULESTORES
+
+    if name not in _MODULESTORES:
+        class_path = settings.MODULESTORE[name]['ENGINE']
+        module_path, _, class_name = class_path.rpartition('.')
+        class_ = getattr(import_module(module_path), class_name)
+        _MODULESTORES[name] = class_(
+            **settings.MODULESTORE[name]['OPTIONS'])
+
+    return _MODULESTORES[name]
diff --git a/common/lib/keystore/exceptions.py b/common/lib/xmodule/modulestore/exceptions.py
similarity index 100%
rename from common/lib/keystore/exceptions.py
rename to common/lib/xmodule/modulestore/exceptions.py
diff --git a/common/lib/keystore/mongo.py b/common/lib/xmodule/modulestore/mongo.py
similarity index 95%
rename from common/lib/keystore/mongo.py
rename to common/lib/xmodule/modulestore/mongo.py
index d92782600c..f305f8aa97 100644
--- a/common/lib/keystore/mongo.py
+++ b/common/lib/xmodule/modulestore/mongo.py
@@ -31,8 +31,8 @@ class MongoModuleStore(ModuleStore):
         recent revision
 
         If any segment of the location is None except revision, raises
-            keystore.exceptions.InsufficientSpecificationError
-        If no object is found at that location, raises keystore.exceptions.ItemNotFoundError
+            xmodule.modulestore.exceptions.InsufficientSpecificationError
+        If no object is found at that location, raises xmodule.modulestore.exceptions.ItemNotFoundError
 
         location: Something that can be passed to Location
         """
diff --git a/common/lib/keystore/tests/test_location.py b/common/lib/xmodule/modulestore/tests/test_location.py
similarity index 95%
rename from common/lib/keystore/tests/test_location.py
rename to common/lib/xmodule/modulestore/tests/test_location.py
index 01d36d946b..d598d8ae6d 100644
--- a/common/lib/keystore/tests/test_location.py
+++ b/common/lib/xmodule/modulestore/tests/test_location.py
@@ -1,6 +1,6 @@
 from nose.tools import assert_equals, assert_raises, assert_not_equals
-from keystore import Location
-from keystore.exceptions import InvalidLocationError
+from xmodule.modulestore import Location
+from xmodule.modulestore.exceptions import InvalidLocationError
 
 
 def check_string_roundtrip(url):
diff --git a/common/lib/keystore/xml.py b/common/lib/xmodule/modulestore/xml.py
similarity index 96%
rename from common/lib/keystore/xml.py
rename to common/lib/xmodule/modulestore/xml.py
index 52eaf0ce0f..d68a6448cc 100644
--- a/common/lib/keystore/xml.py
+++ b/common/lib/xmodule/modulestore/xml.py
@@ -81,8 +81,8 @@ class XMLModuleStore(ModuleStore):
         recent revision
 
         If any segment of the location is None except revision, raises
-            keystore.exceptions.InsufficientSpecificationError
-        If no object is found at that location, raises keystore.exceptions.ItemNotFoundError
+            xmodule.modulestore.exceptions.InsufficientSpecificationError
+        If no object is found at that location, raises xmodule.modulestore.exceptions.ItemNotFoundError
 
         location: Something that can be passed to Location
         """
diff --git a/common/lib/xmodule/tests.py b/common/lib/xmodule/tests/__init__.py
similarity index 100%
rename from common/lib/xmodule/tests.py
rename to common/lib/xmodule/tests/__init__.py
diff --git a/common/lib/xmodule/x_module.py b/common/lib/xmodule/x_module.py
index 8367120748..0af68a2a1a 100644
--- a/common/lib/xmodule/x_module.py
+++ b/common/lib/xmodule/x_module.py
@@ -2,7 +2,7 @@ from lxml import etree
 import pkg_resources
 import logging
 
-from keystore import Location
+from xmodule.modulestore import Location
 from functools import partial
 
 log = logging.getLogger('mitx.' + __name__)
@@ -231,7 +231,7 @@ class XModuleDescriptor(Plugin):
         definition: A dict containing `data` and `children` representing the problem definition
 
         Current arguments passed in kwargs:
-            location: A keystore.Location object indicating the name and ownership of this problem
+            location: A xmodule.modulestore.Location object indicating the name and ownership of this problem
             shared_state_key: The key to use for sharing StudentModules with other
                 modules of this type
             metadata: A dictionary containing the following optional keys:
diff --git a/lms/djangoapps/courseware/management/commands/check_course.py b/lms/djangoapps/courseware/management/commands/check_course.py
index afc7e47857..6ccd6d5fe7 100644
--- a/lms/djangoapps/courseware/management/commands/check_course.py
+++ b/lms/djangoapps/courseware/management/commands/check_course.py
@@ -10,7 +10,7 @@ import xmodule
 
 import mitxmako.middleware as middleware
 middleware.MakoMiddleware()
-from keystore.django import keystore
+from xmodule.modulestore.django import modulestore
 from courseware.models import StudentModuleCache
 from courseware.module_render import get_module
 
@@ -78,7 +78,7 @@ class Command(BaseCommand):
 
         # TODO (cpennington): Get coursename in a legitimate way
         course_location = 'i4x://edx/6002xs12/course/6.002_Spring_2012'
-        student_module_cache = StudentModuleCache(sample_user, keystore().get_item(course_location))
+        student_module_cache = StudentModuleCache(sample_user, modulestore().get_item(course_location))
         (course, _, _, _) = get_module(sample_user, None, course_location, student_module_cache)
 
         to_run = [
diff --git a/lms/djangoapps/courseware/module_render.py b/lms/djangoapps/courseware/module_render.py
index 4e5ee62e63..679084f28c 100644
--- a/lms/djangoapps/courseware/module_render.py
+++ b/lms/djangoapps/courseware/module_render.py
@@ -6,7 +6,7 @@ from django.http import Http404
 from django.http import HttpResponse
 from lxml import etree
 
-from keystore.django import keystore
+from xmodule.modulestore.django import modulestore
 from mitxmako.shortcuts import render_to_string
 from models import StudentModule, StudentModuleCache
 
@@ -129,7 +129,7 @@ def toc_for_course(user, request, course_location, active_chapter, active_sectio
     chapters with name 'hidden' are skipped.
     '''
 
-    student_module_cache = StudentModuleCache(user, keystore().get_item(course_location), depth=2)
+    student_module_cache = StudentModuleCache(user, modulestore().get_item(course_location), depth=2)
     (course, _, _, _) = get_module(user, request, course_location, student_module_cache)
 
     chapters = list()
@@ -161,7 +161,7 @@ def get_section(course, chapter, section):
     section: Section name
     """
     try:
-        course_module = keystore().get_item(course)
+        course_module = modulestore().get_item(course)
     except:
         log.exception("Unable to load course_module")
         return None
@@ -205,7 +205,7 @@ def get_module(user, request, location, student_module_cache, position=None):
         instance_module is a StudentModule specific to this module for this student
         shared_module is a StudentModule specific to all modules with the same 'shared_state_key' attribute, or None if the module doesn't elect to share state
     '''
-    descriptor = keystore().get_item(location)
+    descriptor = modulestore().get_item(location)
 
     instance_module = student_module_cache.lookup(descriptor.category, descriptor.location.url())
     shared_state_key = getattr(descriptor, 'shared_state_key', None)
@@ -304,7 +304,7 @@ def modx_dispatch(request, dispatch=None, id=None):
     # If there are arguments, get rid of them
     dispatch, _, _ = dispatch.partition('?')
 
-    student_module_cache = StudentModuleCache(request.user, keystore().get_item(id))
+    student_module_cache = StudentModuleCache(request.user, modulestore().get_item(id))
     instance, instance_module, shared_module, module_type = get_module(request.user, request, id, student_module_cache)
 
     if instance_module is None:
diff --git a/lms/djangoapps/courseware/views.py b/lms/djangoapps/courseware/views.py
index 52a86fdee4..8b723ca980 100644
--- a/lms/djangoapps/courseware/views.py
+++ b/lms/djangoapps/courseware/views.py
@@ -16,7 +16,7 @@ from module_render import toc_for_course, get_module, get_section
 from models import StudentModuleCache
 from student.models import UserProfile
 from multicourse import multicourse_settings
-from keystore.django import keystore
+from xmodule.modulestore.django import modulestore
 
 from util.cache import cache
 from student.models import UserTestGroup
@@ -63,7 +63,7 @@ def gradebook(request):
     course_location = multicourse_settings.get_course_location(coursename)
 
     for student in student_objects:
-        student_module_cache = StudentModuleCache(student, keystore().get_item(course_location))
+        student_module_cache = StudentModuleCache(student, modulestore().get_item(course_location))
         course, _, _, _ = get_module(request.user, request, course_location, student_module_cache)
         student_info.append({
             'username': student.username,
@@ -93,7 +93,7 @@ def profile(request, student_id=None):
 
     coursename = multicourse_settings.get_coursename_from_request(request)
     course_location = multicourse_settings.get_course_location(coursename)
-    student_module_cache = StudentModuleCache(request.user, keystore().get_item(course_location))
+    student_module_cache = StudentModuleCache(request.user, modulestore().get_item(course_location))
     course, _, _, _ = get_module(request.user, request, course_location, student_module_cache)
 
     context = {'name': user_info.name,
diff --git a/lms/envs/common.py b/lms/envs/common.py
index d1faf00f62..4c3cdc2dda 100644
--- a/lms/envs/common.py
+++ b/lms/envs/common.py
@@ -138,9 +138,9 @@ COURSE_SETTINGS =  {'6.002_Spring_2012': {'number' : '6.002x',
 
 
 ############################### XModule Store ##################################
-KEYSTORE = {
+MODULESTORE = {
     'default': {
-        'ENGINE': 'keystore.xml.XMLModuleStore',
+        'ENGINE': 'xmodule.modulestore.xml.XMLModuleStore',
         'OPTIONS': {
             'org': 'edx',
             'course': '6002xs12',

From 85f294b3e32d9f1e20b7c7775266230b461df767 Mon Sep 17 00:00:00 2001
From: Calen Pennington <calen.pennington@gmail.com>
Date: Mon, 2 Jul 2012 11:21:40 -0400
Subject: [PATCH 24/38] Allow for no default_class in XMLModuleStore

---
 common/lib/xmodule/modulestore/xml.py | 9 ++++++---
 1 file changed, 6 insertions(+), 3 deletions(-)

diff --git a/common/lib/xmodule/modulestore/xml.py b/common/lib/xmodule/modulestore/xml.py
index d68a6448cc..eb23de2e46 100644
--- a/common/lib/xmodule/modulestore/xml.py
+++ b/common/lib/xmodule/modulestore/xml.py
@@ -30,9 +30,12 @@ class XMLModuleStore(ModuleStore):
         self.data_dir = path(data_dir)
         self.modules = {}
 
-        module_path, _, class_name = default_class.rpartition('.')
-        class_ = getattr(import_module(module_path), class_name)
-        self.default_class = class_
+        if default_class is None:
+            self.default_class = None
+        else:
+            module_path, _, class_name = default_class.rpartition('.')
+            class_ = getattr(import_module(module_path), class_name)
+            self.default_class = class_
 
         with open(self.data_dir / "course.xml") as course_file:
             class ImportSystem(XMLParsingSystem):

From d7dbced8e64265625bdd47db8780157a2eefe624 Mon Sep 17 00:00:00 2001
From: Calen Pennington <calen.pennington@gmail.com>
Date: Mon, 2 Jul 2012 11:25:21 -0400
Subject: [PATCH 25/38] Store the top level course in the XMLModuleStore (since
 there is only one course per module store

---
 common/lib/xmodule/modulestore/xml.py | 2 +-
 1 file changed, 1 insertion(+), 1 deletion(-)

diff --git a/common/lib/xmodule/modulestore/xml.py b/common/lib/xmodule/modulestore/xml.py
index eb23de2e46..d4e90d59a4 100644
--- a/common/lib/xmodule/modulestore/xml.py
+++ b/common/lib/xmodule/modulestore/xml.py
@@ -75,7 +75,7 @@ class XMLModuleStore(ModuleStore):
 
                     XMLParsingSystem.__init__(self, modulestore.get_item, OSFS(data_dir), process_xml)
 
-            ImportSystem(self).process_xml(course_file.read())
+            self.course = ImportSystem(self).process_xml(course_file.read())
 
     def get_item(self, location):
         """

From 3a348e5713ddc90037010c5006bb028c2e16fb33 Mon Sep 17 00:00:00 2001
From: Calen Pennington <calen.pennington@gmail.com>
Date: Mon, 2 Jul 2012 11:25:42 -0400
Subject: [PATCH 26/38] Adjust xml export code for the type -> category
 conversion

---
 common/lib/xmodule/xml_module.py | 6 +++---
 1 file changed, 3 insertions(+), 3 deletions(-)

diff --git a/common/lib/xmodule/xml_module.py b/common/lib/xmodule/xml_module.py
index 6daf2bca36..6733fc450c 100644
--- a/common/lib/xmodule/xml_module.py
+++ b/common/lib/xmodule/xml_module.py
@@ -138,8 +138,8 @@ class XmlDescriptor(XModuleDescriptor):
         # Put content in a separate file if it's large (has more than 5 descendent tags)
         if len(list(xml_object.iter())) > 5:
 
-            filepath = self.__class__._format_filepath(self.type, self.name)
-            resource_fs.makedir(self.type, allow_recreate=True)
+            filepath = self.__class__._format_filepath(self.category, self.name)
+            resource_fs.makedir(self.category, allow_recreate=True)
             with resource_fs.open(filepath, 'w') as file:
                 file.write(etree.tostring(xml_object, pretty_print=True))
 
@@ -149,7 +149,7 @@ class XmlDescriptor(XModuleDescriptor):
             xml_object.set('filename', self.name)
 
         xml_object.set('slug', self.name)
-        xml_object.tag = self.type
+        xml_object.tag = self.category
 
         for attr in ('format', 'graceperiod', 'showanswer', 'rerandomize', 'due'):
             if attr in self.metadata and attr not in self._inherited_metadata:

From f035d5602dff8efbabcfa391978a256a3327334f Mon Sep 17 00:00:00 2001
From: Calen Pennington <calen.pennington@gmail.com>
Date: Mon, 2 Jul 2012 12:24:07 -0400
Subject: [PATCH 27/38] Keep abtest children in a consistent order (makes
 testing easier)

---
 common/lib/xmodule/abtest_module.py | 1 +
 1 file changed, 1 insertion(+)

diff --git a/common/lib/xmodule/abtest_module.py b/common/lib/xmodule/abtest_module.py
index f6057171e5..c3e32732f3 100644
--- a/common/lib/xmodule/abtest_module.py
+++ b/common/lib/xmodule/abtest_module.py
@@ -101,6 +101,7 @@ class ABTestDescriptor(RawDescriptor, XmlDescriptor):
             raise InvalidDefinitionError("ABTest portions must add up to less than or equal to 1")
 
         definition['data']['group_portions'][DEFAULT] = default_portion
+        definition['children'].sort()
 
         return definition
 

From c57833dab7ba13166fcd8ced1456297c42c4cd72 Mon Sep 17 00:00:00 2001
From: Calen Pennington <calen.pennington@gmail.com>
Date: Mon, 2 Jul 2012 12:24:19 -0400
Subject: [PATCH 28/38] Define equality for XModuleDescriptors

---
 common/lib/xmodule/raw_module.py      |  1 +
 common/lib/xmodule/template_module.py |  1 +
 common/lib/xmodule/x_module.py        | 25 +++++++++++++++++++++++++
 3 files changed, 27 insertions(+)

diff --git a/common/lib/xmodule/raw_module.py b/common/lib/xmodule/raw_module.py
index 59f28ff4f0..9fe9a9198b 100644
--- a/common/lib/xmodule/raw_module.py
+++ b/common/lib/xmodule/raw_module.py
@@ -3,6 +3,7 @@ from lxml import etree
 from xmodule.mako_module import MakoModuleDescriptor
 from xmodule.xml_module import XmlDescriptor
 
+
 class RawDescriptor(MakoModuleDescriptor, XmlDescriptor):
     """
     Module that provides a raw editing view of it's data and children
diff --git a/common/lib/xmodule/template_module.py b/common/lib/xmodule/template_module.py
index 1057fc2a25..064d48f431 100644
--- a/common/lib/xmodule/template_module.py
+++ b/common/lib/xmodule/template_module.py
@@ -37,5 +37,6 @@ class CustomTagModule(XModule):
     def get_html(self):
         return self.html
 
+
 class CustomTagDescriptor(RawDescriptor):
     module_class = CustomTagModule
diff --git a/common/lib/xmodule/x_module.py b/common/lib/xmodule/x_module.py
index 0af68a2a1a..9f56d95fe5 100644
--- a/common/lib/xmodule/x_module.py
+++ b/common/lib/xmodule/x_module.py
@@ -213,6 +213,10 @@ class XModuleDescriptor(Plugin):
     # A list of metadata that this module can inherit from its parent module
     inheritable_metadata = ('graded', 'due', 'graceperiod', 'showanswer', 'rerandomize')
 
+    # A list of descriptor attributes that must be equal for the discriptors to be
+    # equal
+    equality_attributes = ('definition', 'metadata', 'location', 'shared_state_key', '_inherited_metadata')
+
     # ============================= STRUCTURAL MANIPULATION ===========================
     def __init__(self,
                  system,
@@ -395,6 +399,27 @@ class XModuleDescriptor(Plugin):
         """
         raise NotImplementedError("get_html() must be provided by specific modules")
 
+    # =============================== BUILTIN METHODS ===========================
+    def __eq__(self, other):
+        eq = (self.__class__ == other.__class__ and
+                all(getattr(self, attr, None) == getattr(other, attr, None)
+                    for attr in self.equality_attributes))
+
+        if not eq:
+            for attr in self.equality_attributes:
+                print getattr(self, attr, None), getattr(other, attr, None), getattr(self, attr, None) == getattr(other, attr, None)
+
+        return eq
+
+    def __repr__(self):
+        return "{class_}({system!r}, {definition!r}, location={location!r}, metadata={metadata!r})".format(
+            class_=self.__class__.__name__,
+            system=self.system,
+            definition=self.definition,
+            location=self.location,
+            metadata=self.metadata
+        )
+
 
 class DescriptorSystem(object):
     def __init__(self, load_item, resources_fs):

From be40d8bb6991671a42581e567a5a0a79e914721e Mon Sep 17 00:00:00 2001
From: Calen Pennington <calen.pennington@gmail.com>
Date: Mon, 2 Jul 2012 12:25:09 -0400
Subject: [PATCH 29/38] Make sure that xml_module definition xml doesn't have
 any metadata sprinkled in with it

---
 common/lib/xmodule/xml_module.py | 27 +++++++++++++++++++++++++--
 1 file changed, 25 insertions(+), 2 deletions(-)

diff --git a/common/lib/xmodule/xml_module.py b/common/lib/xmodule/xml_module.py
index 6733fc450c..f860e70afb 100644
--- a/common/lib/xmodule/xml_module.py
+++ b/common/lib/xmodule/xml_module.py
@@ -1,6 +1,7 @@
 from collections import MutableMapping
 from xmodule.x_module import XModuleDescriptor
 from lxml import etree
+import copy
 
 
 class LazyLoadingDict(MutableMapping):
@@ -41,6 +42,10 @@ class LazyLoadingDict(MutableMapping):
         self.load()
         return iter(self._contents)
 
+    def __repr__(self):
+        self.load()
+        return repr(self._contents)
+
     def load(self):
         if self._loaded:
             return
@@ -59,6 +64,11 @@ class XmlDescriptor(XModuleDescriptor):
     # Extension to append to filename paths
     filename_extension = 'xml'
 
+    # 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',
+        'due', 'graded', 'name', 'slug')
+
     @classmethod
     def definition_from_xml(cls, xml_object, system):
         """
@@ -69,6 +79,15 @@ class XmlDescriptor(XModuleDescriptor):
         """
         raise NotImplementedError("%s does not implement definition_from_xml" % cls.__name__)
 
+    @classmethod
+    def clean_metadata_from_xml(cls, xml_object):
+        """
+        Remove any attribute named in self.metadata_attributes from the supplied xml_object
+        """
+        for attr in cls.metadata_attributes:
+            if xml_object.get(attr) is not None:
+                del xml_object.attrib[attr]
+
     @classmethod
     def from_xml(cls, xml_data, system, org=None, course=None):
         """
@@ -101,11 +120,14 @@ class XmlDescriptor(XModuleDescriptor):
         def definition_loader():
             filename = xml_object.get('filename')
             if filename is None:
-                return cls.definition_from_xml(xml_object, system)
+                definition_xml = copy.deepcopy(xml_object)
             else:
                 filepath = cls._format_filepath(xml_object.tag, filename)
                 with system.resources_fs.open(filepath) as file:
-                    return cls.definition_from_xml(etree.parse(file).getroot(), system)
+                    definition_xml = etree.parse(file).getroot()
+
+            cls.clean_metadata_from_xml(definition_xml)
+            return cls.definition_from_xml(definition_xml, system)
 
         return cls(
             system,
@@ -134,6 +156,7 @@ class XmlDescriptor(XModuleDescriptor):
         using the from_xml method with the same system, org, and course
         """
         xml_object = self.definition_to_xml(resource_fs)
+        self.__class__.clean_metadata_from_xml(xml_object)
 
         # Put content in a separate file if it's large (has more than 5 descendent tags)
         if len(list(xml_object.iter())) > 5:

From 8f59521660311adb961a0794e8d22945b854a4ff Mon Sep 17 00:00:00 2001
From: Calen Pennington <calen.pennington@gmail.com>
Date: Mon, 2 Jul 2012 12:25:48 -0400
Subject: [PATCH 30/38] Add a function to check round-trip export/import cycles

---
 common/lib/xmodule/tests/test_export.py | 28 +++++++++++++++++++++++++
 1 file changed, 28 insertions(+)
 create mode 100644 common/lib/xmodule/tests/test_export.py

diff --git a/common/lib/xmodule/tests/test_export.py b/common/lib/xmodule/tests/test_export.py
new file mode 100644
index 0000000000..97da2c4fe5
--- /dev/null
+++ b/common/lib/xmodule/tests/test_export.py
@@ -0,0 +1,28 @@
+from xmodule.modulestore.xml import XMLModuleStore
+from nose.tools import assert_equals
+from tempfile import mkdtemp
+from fs.osfs import OSFS
+
+
+def check_export_roundtrip(data_dir):
+    print "Starting import"
+    initial_import = XMLModuleStore('org', 'course', data_dir, eager=True)
+    initial_course = initial_import.course
+
+    print "Starting export"
+    export_dir = mkdtemp()
+    fs = OSFS(export_dir)
+    xml = initial_course.export_to_xml(fs)
+    with fs.open('course.xml', 'w') as course_xml:
+        course_xml.write(xml)
+
+    print "Starting second import"
+    second_import = XMLModuleStore('org', 'course', export_dir, eager=True)
+
+    print "Checking key equality"
+    assert_equals(initial_import.modules.keys(), second_import.modules.keys())
+
+    print "Checking module equality"
+    for location in initial_import.modules.keys():
+        print "Checking", location
+        assert_equals(initial_import.modules[location], second_import.modules[location])

From cc8ecb1891fd047802c90b9a01d088ad6284860f Mon Sep 17 00:00:00 2001
From: Calen Pennington <calen.pennington@gmail.com>
Date: Mon, 2 Jul 2012 12:50:03 -0400
Subject: [PATCH 31/38] Make xmodule tests pass again

---
 common/lib/xmodule/tests/__init__.py          | 27 +++++++++----------
 .../test_files/formularesponse_with_hint.xml  |  0
 .../{ => tests}/test_files/imageresponse.xml  |  0
 .../{ => tests}/test_files/multi_bare.xml     |  0
 .../{ => tests}/test_files/multichoice.xml    |  0
 .../{ => tests}/test_files/optionresponse.xml |  0
 .../test_files/stringresponse_with_hint.xml   |  0
 .../test_files/symbolicresponse.xml           |  0
 .../{ => tests}/test_files/truefalse.xml      |  0
 9 files changed, 12 insertions(+), 15 deletions(-)
 rename common/lib/xmodule/{ => tests}/test_files/formularesponse_with_hint.xml (100%)
 rename common/lib/xmodule/{ => tests}/test_files/imageresponse.xml (100%)
 rename common/lib/xmodule/{ => tests}/test_files/multi_bare.xml (100%)
 rename common/lib/xmodule/{ => tests}/test_files/multichoice.xml (100%)
 rename common/lib/xmodule/{ => tests}/test_files/optionresponse.xml (100%)
 rename common/lib/xmodule/{ => tests}/test_files/stringresponse_with_hint.xml (100%)
 rename common/lib/xmodule/{ => tests}/test_files/symbolicresponse.xml (100%)
 rename common/lib/xmodule/{ => tests}/test_files/truefalse.xml (100%)

diff --git a/common/lib/xmodule/tests/__init__.py b/common/lib/xmodule/tests/__init__.py
index 90187abc2a..4fb270df13 100644
--- a/common/lib/xmodule/tests/__init__.py
+++ b/common/lib/xmodule/tests/__init__.py
@@ -43,12 +43,10 @@ class ModelsTest(unittest.TestCase):
     def setUp(self):
         pass
 
-    def test_get_module_class(self):
-        vc = xmodule.get_module_class('video')
-        vc_str = "<class 'xmodule.video_module.Module'>"
+    def test_load_class(self):
+        vc = xmodule.x_module.XModuleDescriptor.load_class('video')
+        vc_str = "<class 'xmodule.video_module.VideoDescriptor'>"
         self.assertEqual(str(vc), vc_str)
-        video_id = xmodule.get_default_ids()['video']
-        self.assertEqual(video_id, 'youtube')
 
     def test_calc(self):
         variables={'R1':2.0, 'R3':4.0}
@@ -98,7 +96,7 @@ class ModelsTest(unittest.TestCase):
 class MultiChoiceTest(unittest.TestCase):
     def test_MC_grade(self):
         multichoice_file = os.path.dirname(__file__)+"/test_files/multichoice.xml"
-        test_lcp = lcp.LoncapaProblem(open(multichoice_file), '1', system=i4xs)
+        test_lcp = lcp.LoncapaProblem(open(multichoice_file).read(), '1', system=i4xs)
         correct_answers = {'1_2_1':'choice_foil3'}
         self.assertEquals(test_lcp.grade_answers(correct_answers).get_correctness('1_2_1'), 'correct')
         false_answers = {'1_2_1':'choice_foil2'}
@@ -106,7 +104,7 @@ class MultiChoiceTest(unittest.TestCase):
 
     def test_MC_bare_grades(self):
         multichoice_file = os.path.dirname(__file__)+"/test_files/multi_bare.xml"
-        test_lcp = lcp.LoncapaProblem(open(multichoice_file), '1', system=i4xs)
+        test_lcp = lcp.LoncapaProblem(open(multichoice_file).read(), '1', system=i4xs)
         correct_answers = {'1_2_1':'choice_2'}
         self.assertEquals(test_lcp.grade_answers(correct_answers).get_correctness('1_2_1'), 'correct')
         false_answers = {'1_2_1':'choice_1'}
@@ -114,7 +112,7 @@ class MultiChoiceTest(unittest.TestCase):
         
     def test_TF_grade(self):
         truefalse_file =  os.path.dirname(__file__)+"/test_files/truefalse.xml"
-        test_lcp = lcp.LoncapaProblem(open(truefalse_file), '1', system=i4xs)
+        test_lcp = lcp.LoncapaProblem(open(truefalse_file).read(), '1', system=i4xs)
         correct_answers = {'1_2_1':['choice_foil2', 'choice_foil1']}
         self.assertEquals(test_lcp.grade_answers(correct_answers).get_correctness('1_2_1'), 'correct')
         false_answers = {'1_2_1':['choice_foil1']}
@@ -129,7 +127,7 @@ class MultiChoiceTest(unittest.TestCase):
 class ImageResponseTest(unittest.TestCase):
     def test_ir_grade(self):
         imageresponse_file = os.path.dirname(__file__)+"/test_files/imageresponse.xml"
-        test_lcp = lcp.LoncapaProblem(open(imageresponse_file), '1', system=i4xs)
+        test_lcp = lcp.LoncapaProblem(open(imageresponse_file).read(), '1', system=i4xs)
         correct_answers = {'1_2_1':'(490,11)-(556,98)',
                            '1_2_2':'(242,202)-(296,276)'}
         test_answers = {'1_2_1':'[500,20]',
@@ -142,7 +140,7 @@ class SymbolicResponseTest(unittest.TestCase):
     def test_sr_grade(self):
         raise SkipTest() # This test fails due to dependencies on a local copy of snuggletex-webapp. Until we have figured that out, we'll just skip this test
         symbolicresponse_file = os.path.dirname(__file__)+"/test_files/symbolicresponse.xml"
-        test_lcp = lcp.LoncapaProblem(open(symbolicresponse_file), '1', system=i4xs)
+        test_lcp = lcp.LoncapaProblem(open(symbolicresponse_file).read(), '1', system=i4xs)
         correct_answers = {'1_2_1':'cos(theta)*[[1,0],[0,1]] + i*sin(theta)*[[0,1],[1,0]]',
                            '1_2_1_dynamath': '''
 <math xmlns="http://www.w3.org/1998/Math/MathML">
@@ -235,7 +233,7 @@ class OptionResponseTest(unittest.TestCase):
     '''
     def test_or_grade(self):
         optionresponse_file = os.path.dirname(__file__)+"/test_files/optionresponse.xml"
-        test_lcp = lcp.LoncapaProblem(open(optionresponse_file), '1', system=i4xs)
+        test_lcp = lcp.LoncapaProblem(open(optionresponse_file).read(), '1', system=i4xs)
         correct_answers = {'1_2_1':'True',
                            '1_2_2':'False'}
         test_answers = {'1_2_1':'True',
@@ -251,7 +249,7 @@ class FormulaResponseWithHintTest(unittest.TestCase):
     '''
     def test_or_grade(self):
         problem_file = os.path.dirname(__file__)+"/test_files/formularesponse_with_hint.xml"
-        test_lcp = lcp.LoncapaProblem(open(problem_file), '1', system=i4xs)
+        test_lcp = lcp.LoncapaProblem(open(problem_file).read(), '1', system=i4xs)
         correct_answers = {'1_2_1':'2.5*x-5.0'}
         test_answers = {'1_2_1':'0.4*x-5.0'}
         self.assertEquals(test_lcp.grade_answers(correct_answers).get_correctness('1_2_1'), 'correct')
@@ -265,7 +263,7 @@ class StringResponseWithHintTest(unittest.TestCase):
     '''
     def test_or_grade(self):
         problem_file = os.path.dirname(__file__)+"/test_files/stringresponse_with_hint.xml"
-        test_lcp = lcp.LoncapaProblem(open(problem_file), '1', system=i4xs)
+        test_lcp = lcp.LoncapaProblem(open(problem_file).read(), '1', system=i4xs)
         correct_answers = {'1_2_1':'Michigan'}
         test_answers = {'1_2_1':'Minnesota'}
         self.assertEquals(test_lcp.grade_answers(correct_answers).get_correctness('1_2_1'), 'correct')
@@ -618,7 +616,6 @@ class ModuleProgressTest(unittest.TestCase):
     '''
     def test_xmodule_default(self):
         '''Make sure default get_progress exists, returns None'''
-        xm = x_module.XModule(i4xs, "<dummy />", "dummy")
+        xm = x_module.XModule(i4xs, 'a://b/c/d/e', {})
         p = xm.get_progress()
         self.assertEqual(p, None)
-        
diff --git a/common/lib/xmodule/test_files/formularesponse_with_hint.xml b/common/lib/xmodule/tests/test_files/formularesponse_with_hint.xml
similarity index 100%
rename from common/lib/xmodule/test_files/formularesponse_with_hint.xml
rename to common/lib/xmodule/tests/test_files/formularesponse_with_hint.xml
diff --git a/common/lib/xmodule/test_files/imageresponse.xml b/common/lib/xmodule/tests/test_files/imageresponse.xml
similarity index 100%
rename from common/lib/xmodule/test_files/imageresponse.xml
rename to common/lib/xmodule/tests/test_files/imageresponse.xml
diff --git a/common/lib/xmodule/test_files/multi_bare.xml b/common/lib/xmodule/tests/test_files/multi_bare.xml
similarity index 100%
rename from common/lib/xmodule/test_files/multi_bare.xml
rename to common/lib/xmodule/tests/test_files/multi_bare.xml
diff --git a/common/lib/xmodule/test_files/multichoice.xml b/common/lib/xmodule/tests/test_files/multichoice.xml
similarity index 100%
rename from common/lib/xmodule/test_files/multichoice.xml
rename to common/lib/xmodule/tests/test_files/multichoice.xml
diff --git a/common/lib/xmodule/test_files/optionresponse.xml b/common/lib/xmodule/tests/test_files/optionresponse.xml
similarity index 100%
rename from common/lib/xmodule/test_files/optionresponse.xml
rename to common/lib/xmodule/tests/test_files/optionresponse.xml
diff --git a/common/lib/xmodule/test_files/stringresponse_with_hint.xml b/common/lib/xmodule/tests/test_files/stringresponse_with_hint.xml
similarity index 100%
rename from common/lib/xmodule/test_files/stringresponse_with_hint.xml
rename to common/lib/xmodule/tests/test_files/stringresponse_with_hint.xml
diff --git a/common/lib/xmodule/test_files/symbolicresponse.xml b/common/lib/xmodule/tests/test_files/symbolicresponse.xml
similarity index 100%
rename from common/lib/xmodule/test_files/symbolicresponse.xml
rename to common/lib/xmodule/tests/test_files/symbolicresponse.xml
diff --git a/common/lib/xmodule/test_files/truefalse.xml b/common/lib/xmodule/tests/test_files/truefalse.xml
similarity index 100%
rename from common/lib/xmodule/test_files/truefalse.xml
rename to common/lib/xmodule/tests/test_files/truefalse.xml

From a0f550396cd94ce9cc014ef09a14cd4b3003597d Mon Sep 17 00:00:00 2001
From: Calen Pennington <calen.pennington@gmail.com>
Date: Mon, 2 Jul 2012 13:19:27 -0400
Subject: [PATCH 32/38] Make xmodule fit the typical python installation setup

---
 common/lib/xmodule/setup.py                                     | 2 +-
 common/lib/xmodule/{ => xmodule}/__init__.py                    | 0
 common/lib/xmodule/{ => xmodule}/abtest_module.py               | 0
 common/lib/xmodule/{ => xmodule}/capa_module.py                 | 0
 common/lib/xmodule/{ => xmodule}/exceptions.py                  | 0
 common/lib/xmodule/{ => xmodule}/graders.py                     | 0
 common/lib/xmodule/{ => xmodule}/hidden_module.py               | 0
 common/lib/xmodule/{ => xmodule}/html_module.py                 | 0
 common/lib/xmodule/{ => xmodule}/js/module/html.coffee          | 0
 common/lib/xmodule/{ => xmodule}/js/module/raw.coffee           | 0
 common/lib/xmodule/{ => xmodule}/mako_module.py                 | 0
 common/lib/xmodule/{ => xmodule}/modulestore/__init__.py        | 0
 common/lib/xmodule/{ => xmodule}/modulestore/django.py          | 0
 common/lib/xmodule/{ => xmodule}/modulestore/exceptions.py      | 0
 common/lib/xmodule/{ => xmodule}/modulestore/mongo.py           | 0
 .../xmodule/{ => xmodule}/modulestore/tests/test_location.py    | 0
 common/lib/xmodule/{ => xmodule}/modulestore/xml.py             | 0
 common/lib/xmodule/{ => xmodule}/progress.py                    | 0
 common/lib/xmodule/{ => xmodule}/raw_module.py                  | 0
 common/lib/xmodule/{ => xmodule}/schematic_module.py            | 0
 common/lib/xmodule/{ => xmodule}/seq_module.py                  | 0
 common/lib/xmodule/{ => xmodule}/template_module.py             | 0
 common/lib/xmodule/{ => xmodule}/translation_module.py          | 0
 common/lib/xmodule/{ => xmodule}/vertical_module.py             | 0
 common/lib/xmodule/{ => xmodule}/video_module.py                | 0
 common/lib/xmodule/{ => xmodule}/x_module.py                    | 0
 common/lib/xmodule/{ => xmodule}/xml_module.py                  | 0
 27 files changed, 1 insertion(+), 1 deletion(-)
 rename common/lib/xmodule/{ => xmodule}/__init__.py (100%)
 rename common/lib/xmodule/{ => xmodule}/abtest_module.py (100%)
 rename common/lib/xmodule/{ => xmodule}/capa_module.py (100%)
 rename common/lib/xmodule/{ => xmodule}/exceptions.py (100%)
 rename common/lib/xmodule/{ => xmodule}/graders.py (100%)
 rename common/lib/xmodule/{ => xmodule}/hidden_module.py (100%)
 rename common/lib/xmodule/{ => xmodule}/html_module.py (100%)
 rename common/lib/xmodule/{ => xmodule}/js/module/html.coffee (100%)
 rename common/lib/xmodule/{ => xmodule}/js/module/raw.coffee (100%)
 rename common/lib/xmodule/{ => xmodule}/mako_module.py (100%)
 rename common/lib/xmodule/{ => xmodule}/modulestore/__init__.py (100%)
 rename common/lib/xmodule/{ => xmodule}/modulestore/django.py (100%)
 rename common/lib/xmodule/{ => xmodule}/modulestore/exceptions.py (100%)
 rename common/lib/xmodule/{ => xmodule}/modulestore/mongo.py (100%)
 rename common/lib/xmodule/{ => xmodule}/modulestore/tests/test_location.py (100%)
 rename common/lib/xmodule/{ => xmodule}/modulestore/xml.py (100%)
 rename common/lib/xmodule/{ => xmodule}/progress.py (100%)
 rename common/lib/xmodule/{ => xmodule}/raw_module.py (100%)
 rename common/lib/xmodule/{ => xmodule}/schematic_module.py (100%)
 rename common/lib/xmodule/{ => xmodule}/seq_module.py (100%)
 rename common/lib/xmodule/{ => xmodule}/template_module.py (100%)
 rename common/lib/xmodule/{ => xmodule}/translation_module.py (100%)
 rename common/lib/xmodule/{ => xmodule}/vertical_module.py (100%)
 rename common/lib/xmodule/{ => xmodule}/video_module.py (100%)
 rename common/lib/xmodule/{ => xmodule}/x_module.py (100%)
 rename common/lib/xmodule/{ => xmodule}/xml_module.py (100%)

diff --git a/common/lib/xmodule/setup.py b/common/lib/xmodule/setup.py
index e45e6654c2..3a380b12e7 100644
--- a/common/lib/xmodule/setup.py
+++ b/common/lib/xmodule/setup.py
@@ -3,7 +3,7 @@ from setuptools import setup, find_packages
 setup(
     name="XModule",
     version="0.1",
-    packages=find_packages(),
+    packages=find_packages(exclude=["tests"]),
     install_requires=['distribute'],
     package_data={
         '': ['js/*']
diff --git a/common/lib/xmodule/__init__.py b/common/lib/xmodule/xmodule/__init__.py
similarity index 100%
rename from common/lib/xmodule/__init__.py
rename to common/lib/xmodule/xmodule/__init__.py
diff --git a/common/lib/xmodule/abtest_module.py b/common/lib/xmodule/xmodule/abtest_module.py
similarity index 100%
rename from common/lib/xmodule/abtest_module.py
rename to common/lib/xmodule/xmodule/abtest_module.py
diff --git a/common/lib/xmodule/capa_module.py b/common/lib/xmodule/xmodule/capa_module.py
similarity index 100%
rename from common/lib/xmodule/capa_module.py
rename to common/lib/xmodule/xmodule/capa_module.py
diff --git a/common/lib/xmodule/exceptions.py b/common/lib/xmodule/xmodule/exceptions.py
similarity index 100%
rename from common/lib/xmodule/exceptions.py
rename to common/lib/xmodule/xmodule/exceptions.py
diff --git a/common/lib/xmodule/graders.py b/common/lib/xmodule/xmodule/graders.py
similarity index 100%
rename from common/lib/xmodule/graders.py
rename to common/lib/xmodule/xmodule/graders.py
diff --git a/common/lib/xmodule/hidden_module.py b/common/lib/xmodule/xmodule/hidden_module.py
similarity index 100%
rename from common/lib/xmodule/hidden_module.py
rename to common/lib/xmodule/xmodule/hidden_module.py
diff --git a/common/lib/xmodule/html_module.py b/common/lib/xmodule/xmodule/html_module.py
similarity index 100%
rename from common/lib/xmodule/html_module.py
rename to common/lib/xmodule/xmodule/html_module.py
diff --git a/common/lib/xmodule/js/module/html.coffee b/common/lib/xmodule/xmodule/js/module/html.coffee
similarity index 100%
rename from common/lib/xmodule/js/module/html.coffee
rename to common/lib/xmodule/xmodule/js/module/html.coffee
diff --git a/common/lib/xmodule/js/module/raw.coffee b/common/lib/xmodule/xmodule/js/module/raw.coffee
similarity index 100%
rename from common/lib/xmodule/js/module/raw.coffee
rename to common/lib/xmodule/xmodule/js/module/raw.coffee
diff --git a/common/lib/xmodule/mako_module.py b/common/lib/xmodule/xmodule/mako_module.py
similarity index 100%
rename from common/lib/xmodule/mako_module.py
rename to common/lib/xmodule/xmodule/mako_module.py
diff --git a/common/lib/xmodule/modulestore/__init__.py b/common/lib/xmodule/xmodule/modulestore/__init__.py
similarity index 100%
rename from common/lib/xmodule/modulestore/__init__.py
rename to common/lib/xmodule/xmodule/modulestore/__init__.py
diff --git a/common/lib/xmodule/modulestore/django.py b/common/lib/xmodule/xmodule/modulestore/django.py
similarity index 100%
rename from common/lib/xmodule/modulestore/django.py
rename to common/lib/xmodule/xmodule/modulestore/django.py
diff --git a/common/lib/xmodule/modulestore/exceptions.py b/common/lib/xmodule/xmodule/modulestore/exceptions.py
similarity index 100%
rename from common/lib/xmodule/modulestore/exceptions.py
rename to common/lib/xmodule/xmodule/modulestore/exceptions.py
diff --git a/common/lib/xmodule/modulestore/mongo.py b/common/lib/xmodule/xmodule/modulestore/mongo.py
similarity index 100%
rename from common/lib/xmodule/modulestore/mongo.py
rename to common/lib/xmodule/xmodule/modulestore/mongo.py
diff --git a/common/lib/xmodule/modulestore/tests/test_location.py b/common/lib/xmodule/xmodule/modulestore/tests/test_location.py
similarity index 100%
rename from common/lib/xmodule/modulestore/tests/test_location.py
rename to common/lib/xmodule/xmodule/modulestore/tests/test_location.py
diff --git a/common/lib/xmodule/modulestore/xml.py b/common/lib/xmodule/xmodule/modulestore/xml.py
similarity index 100%
rename from common/lib/xmodule/modulestore/xml.py
rename to common/lib/xmodule/xmodule/modulestore/xml.py
diff --git a/common/lib/xmodule/progress.py b/common/lib/xmodule/xmodule/progress.py
similarity index 100%
rename from common/lib/xmodule/progress.py
rename to common/lib/xmodule/xmodule/progress.py
diff --git a/common/lib/xmodule/raw_module.py b/common/lib/xmodule/xmodule/raw_module.py
similarity index 100%
rename from common/lib/xmodule/raw_module.py
rename to common/lib/xmodule/xmodule/raw_module.py
diff --git a/common/lib/xmodule/schematic_module.py b/common/lib/xmodule/xmodule/schematic_module.py
similarity index 100%
rename from common/lib/xmodule/schematic_module.py
rename to common/lib/xmodule/xmodule/schematic_module.py
diff --git a/common/lib/xmodule/seq_module.py b/common/lib/xmodule/xmodule/seq_module.py
similarity index 100%
rename from common/lib/xmodule/seq_module.py
rename to common/lib/xmodule/xmodule/seq_module.py
diff --git a/common/lib/xmodule/template_module.py b/common/lib/xmodule/xmodule/template_module.py
similarity index 100%
rename from common/lib/xmodule/template_module.py
rename to common/lib/xmodule/xmodule/template_module.py
diff --git a/common/lib/xmodule/translation_module.py b/common/lib/xmodule/xmodule/translation_module.py
similarity index 100%
rename from common/lib/xmodule/translation_module.py
rename to common/lib/xmodule/xmodule/translation_module.py
diff --git a/common/lib/xmodule/vertical_module.py b/common/lib/xmodule/xmodule/vertical_module.py
similarity index 100%
rename from common/lib/xmodule/vertical_module.py
rename to common/lib/xmodule/xmodule/vertical_module.py
diff --git a/common/lib/xmodule/video_module.py b/common/lib/xmodule/xmodule/video_module.py
similarity index 100%
rename from common/lib/xmodule/video_module.py
rename to common/lib/xmodule/xmodule/video_module.py
diff --git a/common/lib/xmodule/x_module.py b/common/lib/xmodule/xmodule/x_module.py
similarity index 100%
rename from common/lib/xmodule/x_module.py
rename to common/lib/xmodule/xmodule/x_module.py
diff --git a/common/lib/xmodule/xml_module.py b/common/lib/xmodule/xmodule/xml_module.py
similarity index 100%
rename from common/lib/xmodule/xml_module.py
rename to common/lib/xmodule/xmodule/xml_module.py

From e56f8763ac35261a366518d69487679675c9d66e Mon Sep 17 00:00:00 2001
From: Calen Pennington <calen.pennington@gmail.com>
Date: Mon, 2 Jul 2012 13:26:14 -0400
Subject: [PATCH 33/38] Point to the js files in package data

---
 common/lib/xmodule/setup.py | 2 +-
 1 file changed, 1 insertion(+), 1 deletion(-)

diff --git a/common/lib/xmodule/setup.py b/common/lib/xmodule/setup.py
index 3a380b12e7..77b0838ff2 100644
--- a/common/lib/xmodule/setup.py
+++ b/common/lib/xmodule/setup.py
@@ -6,7 +6,7 @@ setup(
     packages=find_packages(exclude=["tests"]),
     install_requires=['distribute'],
     package_data={
-        '': ['js/*']
+        'xmodule': ['js/module/*']
     },
 
     # See http://guide.python-distribute.org/creation.html#entry-points

From d7178e4a41d00573459bf44555c1cff66798273c Mon Sep 17 00:00:00 2001
From: Calen Pennington <calen.pennington@gmail.com>
Date: Mon, 2 Jul 2012 13:26:31 -0400
Subject: [PATCH 34/38] Add a set of rake tasks for checking settings
 importability

---
 rakefile | 8 ++++++++
 1 file changed, 8 insertions(+)

diff --git a/rakefile b/rakefile
index e76e200777..f5e32d8110 100644
--- a/rakefile
+++ b/rakefile
@@ -84,6 +84,14 @@ default_options = {
         args.with_defaults(:env => 'dev', :options => default_options[system])
         sh(django_admin(system, args.env, 'runserver', args.options))
     end
+    
+    Dir["#{system}/envs/*.py"].each do |env_file|
+        env = File.basename(env_file).gsub(/\.py/, '')
+        desc "Attempt to import the settings file #{system}.envs.#{env} and report any errors"
+        task "#{system}:check_settings:#{env}" do
+            sh("echo 'import #{system}.envs.#{env}' | #{django_admin(system, env, 'shell')}")
+        end
+    end
 end
 
 Dir["common/lib/*"].each do |lib|

From 3b4fb616488a1eab7b3d135d26bf4b0d1ffcdc0d Mon Sep 17 00:00:00 2001
From: Calen Pennington <calen.pennington@gmail.com>
Date: Mon, 2 Jul 2012 14:10:29 -0400
Subject: [PATCH 35/38] Push dependency on mitxmako up out of mako_module

---
 common/lib/xmodule/xmodule/mako_module.py       | 16 +++++++++++++---
 common/lib/xmodule/xmodule/modulestore/mongo.py |  6 ++++--
 2 files changed, 17 insertions(+), 5 deletions(-)

diff --git a/common/lib/xmodule/xmodule/mako_module.py b/common/lib/xmodule/xmodule/mako_module.py
index 2260dddd92..9a90afb896 100644
--- a/common/lib/xmodule/xmodule/mako_module.py
+++ b/common/lib/xmodule/xmodule/mako_module.py
@@ -1,5 +1,10 @@
-from x_module import XModuleDescriptor
-from mitxmako.shortcuts import render_to_string
+from x_module import XModuleDescriptor, DescriptorSystem
+
+
+class MakoDescriptorSystem(DescriptorSystem):
+    def __init__(self, render_template, *args, **kwargs):
+        self.render_template = render_template
+        super(MakoDescriptorSystem, self).__init__(*args, **kwargs)
 
 
 class MakoModuleDescriptor(XModuleDescriptor):
@@ -12,6 +17,11 @@ class MakoModuleDescriptor(XModuleDescriptor):
     the descriptor as the `module` parameter to that template
     """
 
+    def __init__(self, system, definition=None, **kwargs):
+        if getattr(system, 'render_template', None) is None:
+            raise TypeError('{system} must have a render_template function in order to use a MakoDescriptor'.format(system=system))
+        super(MakoModuleDescriptor, self).__init__(system, definition, **kwargs)
+
     def get_context(self):
         """
         Return the context to render the mako template with
@@ -19,4 +29,4 @@ class MakoModuleDescriptor(XModuleDescriptor):
         return {'module': self}
 
     def get_html(self):
-        return render_to_string(self.mako_template, self.get_context())
+        return self.system.render_template(self.mako_template, self.get_context())
diff --git a/common/lib/xmodule/xmodule/modulestore/mongo.py b/common/lib/xmodule/xmodule/modulestore/mongo.py
index f305f8aa97..cc731c929c 100644
--- a/common/lib/xmodule/xmodule/modulestore/mongo.py
+++ b/common/lib/xmodule/xmodule/modulestore/mongo.py
@@ -1,6 +1,8 @@
 import pymongo
 from importlib import import_module
-from xmodule.x_module import XModuleDescriptor, DescriptorSystem
+from xmodule.x_module import XModuleDescriptor
+from xmodule.mako_module import MakoDescriptorSystem
+from mitxmako.shortcuts import render_to_string
 
 from . import ModuleStore, Location
 from .exceptions import ItemNotFoundError, InsufficientSpecificationError
@@ -54,7 +56,7 @@ class MongoModuleStore(ModuleStore):
 
         # TODO (cpennington): Pass a proper resources_fs to the system
         return XModuleDescriptor.load_from_json(
-            item, DescriptorSystem(self.get_item, None), self.default_class)
+            item, MakoDescriptorSystem(load_item=self.get_item, resources_fs=None, render_template=render_to_string), self.default_class)
 
     def create_item(self, location):
         """

From e172be3a26dd06b8fdd657d05ccee3d2f0c75ae9 Mon Sep 17 00:00:00 2001
From: Calen Pennington <calen.pennington@gmail.com>
Date: Mon, 2 Jul 2012 19:55:42 -0400
Subject: [PATCH 36/38] Make XML import pass in an empty render_template
 function

---
 common/lib/xmodule/xmodule/modulestore/xml.py | 12 ++++++++++--
 common/lib/xmodule/xmodule/x_module.py        |  4 ++--
 2 files changed, 12 insertions(+), 4 deletions(-)

diff --git a/common/lib/xmodule/xmodule/modulestore/xml.py b/common/lib/xmodule/xmodule/modulestore/xml.py
index d4e90d59a4..a5db17054b 100644
--- a/common/lib/xmodule/xmodule/modulestore/xml.py
+++ b/common/lib/xmodule/xmodule/modulestore/xml.py
@@ -4,6 +4,7 @@ from importlib import import_module
 from lxml import etree
 from path import path
 from xmodule.x_module import XModuleDescriptor, XMLParsingSystem
+from xmodule.mako_module import MakoDescriptorSystem
 
 from . import ModuleStore, Location
 from .exceptions import ItemNotFoundError
@@ -38,7 +39,7 @@ class XMLModuleStore(ModuleStore):
             self.default_class = class_
 
         with open(self.data_dir / "course.xml") as course_file:
-            class ImportSystem(XMLParsingSystem):
+            class ImportSystem(XMLParsingSystem, MakoDescriptorSystem):
                 def __init__(self, modulestore):
                     """
                     modulestore: the XMLModuleStore to store the loaded modules in
@@ -73,7 +74,14 @@ class XMLModuleStore(ModuleStore):
                             module.get_children()
                         return module
 
-                    XMLParsingSystem.__init__(self, modulestore.get_item, OSFS(data_dir), process_xml)
+                    system_kwargs = dict(
+                        render_template=lambda: '',
+                        load_item=modulestore.get_item,
+                        resources_fs=OSFS(data_dir),
+                        process_xml=process_xml
+                    )
+                    MakoDescriptorSystem.__init__(self, **system_kwargs)
+                    XMLParsingSystem.__init__(self, **system_kwargs)
 
             self.course = ImportSystem(self).process_xml(course_file.read())
 
diff --git a/common/lib/xmodule/xmodule/x_module.py b/common/lib/xmodule/xmodule/x_module.py
index 9f56d95fe5..8bfbb5f91a 100644
--- a/common/lib/xmodule/xmodule/x_module.py
+++ b/common/lib/xmodule/xmodule/x_module.py
@@ -422,7 +422,7 @@ class XModuleDescriptor(Plugin):
 
 
 class DescriptorSystem(object):
-    def __init__(self, load_item, resources_fs):
+    def __init__(self, load_item, resources_fs, **kwargs):
         """
         load_item: Takes a Location and returns an XModuleDescriptor
         resources_fs: A Filesystem object that contains all of the
@@ -434,7 +434,7 @@ class DescriptorSystem(object):
 
 
 class XMLParsingSystem(DescriptorSystem):
-    def __init__(self, load_item, resources_fs, process_xml):
+    def __init__(self, load_item, resources_fs, process_xml, **kwargs):
         """
         process_xml: Takes an xml string, and returns the the XModuleDescriptor created from that xml
         """

From 3355f804d100d6df9eb54b2d2ff8bac5f1b5c303 Mon Sep 17 00:00:00 2001
From: Calen Pennington <calen.pennington@gmail.com>
Date: Mon, 2 Jul 2012 20:01:01 -0400
Subject: [PATCH 37/38] Add logging of filename when module file parsing fails

---
 cms/envs/dev.py                          | 4 ++++
 common/lib/xmodule/xmodule/xml_module.py | 8 +++++++-
 2 files changed, 11 insertions(+), 1 deletion(-)

diff --git a/cms/envs/dev.py b/cms/envs/dev.py
index 5bc5cfebc5..b4bcbfa9ce 100644
--- a/cms/envs/dev.py
+++ b/cms/envs/dev.py
@@ -3,6 +3,10 @@ This config file runs the simplest dev environment"""
 
 from .common import *
 
+import logging
+import sys
+logging.basicConfig(stream=sys.stdout, )
+
 DEBUG = True
 TEMPLATE_DEBUG = DEBUG
 
diff --git a/common/lib/xmodule/xmodule/xml_module.py b/common/lib/xmodule/xmodule/xml_module.py
index f860e70afb..aebb024a59 100644
--- a/common/lib/xmodule/xmodule/xml_module.py
+++ b/common/lib/xmodule/xmodule/xml_module.py
@@ -2,7 +2,9 @@ from collections import MutableMapping
 from xmodule.x_module import XModuleDescriptor
 from lxml import etree
 import copy
+import logging
 
+log = logging.getLogger(__name__)
 
 class LazyLoadingDict(MutableMapping):
     """
@@ -124,7 +126,11 @@ class XmlDescriptor(XModuleDescriptor):
             else:
                 filepath = cls._format_filepath(xml_object.tag, filename)
                 with system.resources_fs.open(filepath) as file:
-                    definition_xml = etree.parse(file).getroot()
+                    try:
+                        definition_xml = etree.parse(file).getroot()
+                    except:
+                        log.exception("Failed to parse xml in file %s" % filepath)
+                        raise
 
             cls.clean_metadata_from_xml(definition_xml)
             return cls.definition_from_xml(definition_xml, system)

From 64a4a62cf53dbb2d871999f142e76ce62dd1957a Mon Sep 17 00:00:00 2001
From: Calen Pennington <calen.pennington@gmail.com>
Date: Mon, 2 Jul 2012 20:01:20 -0400
Subject: [PATCH 38/38] Don't pass fileobjects to LoncapaProblem

---
 common/lib/xmodule/xmodule/capa_module.py | 11 +++++------
 1 file changed, 5 insertions(+), 6 deletions(-)

diff --git a/common/lib/xmodule/xmodule/capa_module.py b/common/lib/xmodule/xmodule/capa_module.py
index 57c5fa88ce..b7f8e68b5e 100644
--- a/common/lib/xmodule/xmodule/capa_module.py
+++ b/common/lib/xmodule/xmodule/capa_module.py
@@ -141,9 +141,8 @@ class CapaModule(XModule):
                 msg = '<p>%s</p>' % msg.replace('<', '&lt;')
                 msg += '<p><pre>%s</pre></p>' % traceback.format_exc().replace('<', '&lt;')
                 # create a dummy problem with error message instead of failing
-                fp = StringIO.StringIO('<problem><text><font color="red" size="+2">Problem file %s has an error:</font>%s</text></problem>' % (self.filename, msg))
-                fp.name = "StringIO"
-                self.lcp = LoncapaProblem(fp, self.location.html_id(), instance_state, seed=seed, system=self.system)
+                problem_text = '<problem><text><font color="red" size="+2">Problem file %s has an error:</font>%s</text></problem>' % (self.filename, msg)
+                self.lcp = LoncapaProblem(problem_text, self.location.html_id(), instance_state, seed=seed, system=self.system)
             else:
                 raise
 
@@ -395,13 +394,13 @@ class CapaModule(XModule):
             correct_map = self.lcp.grade_answers(answers)
         except StudentInputError as inst:
             # TODO (vshnayder): why is this line here?
-            self.lcp = LoncapaProblem(self.system.filestore.open(self.filename),
+            self.lcp = LoncapaProblem(self.system.filestore.open(self.filename).read(),
                                       id=lcp_id, state=old_state, system=self.system)
             traceback.print_exc()
             return {'success': inst.message}
         except:
             # TODO: why is this line here?
-            self.lcp = LoncapaProblem(self.system.filestore.open(self.filename),
+            self.lcp = LoncapaProblem(self.system.filestore.open(self.filename).read(),
                                       id=lcp_id, state=old_state, system=self.system)
             traceback.print_exc()
             raise Exception("error in capa_module")
@@ -486,7 +485,7 @@ class CapaModule(XModule):
             # reset random number generator seed (note the self.lcp.get_state() in next line)
             self.lcp.seed = None
 
-        self.lcp = LoncapaProblem(self.system.filestore.open(self.filename),
+        self.lcp = LoncapaProblem(self.system.filestore.open(self.filename).read(),
                                   self.location.html_id(), self.lcp.get_state(), system=self.system)
 
         event_info['new_state'] = self.lcp.get_state()