From 26ae88faac8c620fa14d0cabfee7c03ac5938169 Mon Sep 17 00:00:00 2001 From: Victor Shnayder Date: Thu, 2 Aug 2012 13:41:54 -0400 Subject: [PATCH] Factor out get_item_error() into a new ModuleStoreBase class * may be temporary if we move errors into the items themselves. --- .../xmodule/xmodule/modulestore/__init__.py | 44 +++++++++++++++++-- .../lib/xmodule/xmodule/modulestore/mongo.py | 9 ++-- common/lib/xmodule/xmodule/modulestore/xml.py | 36 +++++---------- 3 files changed, 59 insertions(+), 30 deletions(-) diff --git a/common/lib/xmodule/xmodule/modulestore/__init__.py b/common/lib/xmodule/xmodule/modulestore/__init__.py index 7241179d8e..e59e4bd68e 100644 --- a/common/lib/xmodule/xmodule/modulestore/__init__.py +++ b/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 re -from collections import namedtuple -from .exceptions import InvalidLocationError, InsufficientSpecificationError import logging +import re + +from collections import namedtuple + +from .exceptions import InvalidLocationError, InsufficientSpecificationError +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 diff --git a/common/lib/xmodule/xmodule/modulestore/mongo.py b/common/lib/xmodule/xmodule/modulestore/mongo.py index 76769b25b0..1cec6c7f87 100644 --- a/common/lib/xmodule/xmodule/modulestore/mongo.py +++ b/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 diff --git a/common/lib/xmodule/xmodule/modulestore/xml.py b/common/lib/xmodule/xmodule/modulestore/xml.py index d6540023e8..0567e4e7a7 100644 --- a/common/lib/xmodule/xmodule/modulestore/xml.py +++ b/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