Factor out get_item_error() into a new ModuleStoreBase class

* may be temporary if we move errors into the items themselves.

common/lib/xmodule/xmodule/modulestore/__init__.py

@@ -3,10 +3,13 @@ This module provides an abstraction for working with XModuleDescriptors
 that are stored in a database an accessible using their Location as an identifier
 """
 
+import logging
 import re
+
 from collections import namedtuple
+
 from .exceptions import InvalidLocationError, InsufficientSpecificationError
-import logging
+from xmodule.errortracker import ErrorLog, make_error_tracker
 
 log = logging.getLogger('mitx.' + 'modulestore')
 
@@ -290,3 +293,38 @@ class ModuleStore(object):
         '''
         raise NotImplementedError
 
+
+class ModuleStoreBase(ModuleStore):
+    '''
+    Implement interface functionality that can be shared.
+    '''
+    def __init__(self):
+        '''
+        Set up the error-tracking logic.
+        '''
+        self._location_errors = {}    # location -> ErrorLog
+
+    def _get_errorlog(self, location):
+        """
+        If we already have an errorlog for this location, return it.  Otherwise,
+        create one.
+        """
+        location = Location(location)
+        if location not in self._location_errors:
+            self._location_errors[location] = make_error_tracker()
+        return self._location_errors[location]
+
+    def get_item_errors(self, location):
+        """
+        Return list of errors for this location, if any.  Raise the same
+        errors as get_item if location isn't present.
+
+        NOTE: For now, the only items that track errors are CourseDescriptors in
+        the xml datastore.  This will return an empty list for all other items
+        and datastores.
+        """
+        # check that item is present and raise the promised exceptions if needed
+        self.get_item(location)
+
+        errorlog = self._get_errorlog(location)
+        return errorlog.errors

common/lib/xmodule/xmodule/modulestore/mongo.py

@@ -11,7 +11,7 @@ from xmodule.x_module import XModuleDescriptor
 from xmodule.mako_module import MakoDescriptorSystem
 from mitxmako.shortcuts import render_to_string
 
-from . import ModuleStore, Location
+from . import ModuleStoreBase, Location
 from .exceptions import (ItemNotFoundError,
                          NoPathToItem, DuplicateItemError)
 
@@ -38,7 +38,7 @@ class CachingDescriptorSystem(MakoDescriptorSystem):
 
         resources_fs: a filesystem, as per MakoDescriptorSystem
 
-        error_tracker:
+        error_tracker: a function that logs errors for later display to users
 
         render_template: a function for rendering templates, as per
             MakoDescriptorSystem
@@ -73,7 +73,7 @@ def location_to_query(location):
     return query
 
 
-class MongoModuleStore(ModuleStore):
+class MongoModuleStore(ModuleStoreBase):
     """
     A Mongodb backed ModuleStore
     """
@@ -81,6 +81,9 @@ class MongoModuleStore(ModuleStore):
     # TODO (cpennington): Enable non-filesystem filestores
     def __init__(self, host, db, collection, fs_root, port=27017, default_class=None,
                  error_tracker=null_error_tracker):
+
+        ModuleStoreBase.__init__(self)
+
         self.collection = pymongo.connection.Connection(
             host=host,
             port=port

common/lib/xmodule/xmodule/modulestore/xml.py

@@ -12,7 +12,7 @@ from xmodule.course_module import CourseDescriptor
 from xmodule.mako_module import MakoDescriptorSystem
 from cStringIO import StringIO
 
-from . import ModuleStore, Location
+from . import ModuleStoreBase, Location
 from .exceptions import ItemNotFoundError
 
 etree.set_default_parser(
@@ -98,7 +98,7 @@ class ImportSystem(XMLParsingSystem, MakoDescriptorSystem):
                                   error_tracker, process_xml, **kwargs)
 
 
-class XMLModuleStore(ModuleStore):
+class XMLModuleStore(ModuleStoreBase):
     """
     An XML backed ModuleStore
     """
@@ -118,13 +118,12 @@ class XMLModuleStore(ModuleStore):
         course_dirs: If specified, the list of course_dirs to load. Otherwise,
             load all course dirs
         """
+        ModuleStoreBase.__init__(self)
 
         self.eager = eager
         self.data_dir = path(data_dir)
         self.modules = {}  # location -> XModuleDescriptor
         self.courses = {}  # course_dir -> XModuleDescriptor for the course
-        self.location_errors = {}    # location -> ErrorLog
-
 
         if default_class is None:
             self.default_class = None
@@ -148,12 +147,14 @@ class XMLModuleStore(ModuleStore):
 
         for course_dir in course_dirs:
             try:
-                # make a tracker, then stick in the right place once the course loads
-                # and we know its location
+                # Special-case code here, since we don't have a location for the
+                # course before it loads.
+                # So, make a tracker to track load-time errors, then put in the right
+                # place after the course loads and we have its location
                 errorlog = make_error_tracker()
                 course_descriptor = self.load_course(course_dir, errorlog.tracker)
                 self.courses[course_dir] = course_descriptor
-                self.location_errors[course_descriptor.location] = errorlog
+                self._location_errors[course_descriptor.location] = errorlog
             except:
                 msg = "Failed to load course '%s'" % course_dir
                 log.exception(msg)
@@ -221,23 +222,6 @@ class XMLModuleStore(ModuleStore):
             raise ItemNotFoundError(location)
 
 
-    def get_item_errors(self, location):
-        """
-        Return list of errors for this location, if any.  Raise the same
-        errors as get_item if location isn't present.
-
-        NOTE: This only actually works for courses in the xml datastore--
-        will return an empty list for all other modules.
-        """
-        location = Location(location)
-        # check that item is present
-        self.get_item(location)
-
-        # now look up errors
-        if location in self.location_errors:
-            return self.location_errors[location].errors
-        return []
-
     def get_courses(self, depth=0):
         """
         Returns a list of course descriptors.  If there were errors on loading,
@@ -245,9 +229,11 @@ class XMLModuleStore(ModuleStore):
         """
         return self.courses.values()
 
+
     def create_item(self, location):
         raise NotImplementedError("XMLModuleStores are read-only")
 
+
     def update_item(self, location, data):
         """
         Set the data in the item specified by the location to
@@ -258,6 +244,7 @@ class XMLModuleStore(ModuleStore):
         """
         raise NotImplementedError("XMLModuleStores are read-only")
 
+
     def update_children(self, location, children):
         """
         Set the children for the item specified by the location to
@@ -268,6 +255,7 @@ class XMLModuleStore(ModuleStore):
         """
         raise NotImplementedError("XMLModuleStores are read-only")
 
+
     def update_metadata(self, location, metadata):
         """
         Set the metadata for the item specified by the location to