diff --git a/common/lib/xmodule/xmodule/html_module.py b/common/lib/xmodule/xmodule/html_module.py
index 22d3bf163f..996d3c4ead 100644
--- a/common/lib/xmodule/xmodule/html_module.py
+++ b/common/lib/xmodule/xmodule/html_module.py
@@ -54,7 +54,7 @@ class HtmlDescriptor(XmlDescriptor, EditingDescriptor):
# snippets that will be included in the middle of pages.
@classmethod
- def definition_loader(cls, xml_object, system):
+ def load_definition(cls, xml_object, system, location):
'''Load a descriptor from the specified xml_object:
If there is a filename attribute, load it as a string, and
diff --git a/common/lib/xmodule/xmodule/modulestore/__init__.py b/common/lib/xmodule/xmodule/modulestore/__init__.py
index 279782b61a..a0167d781b 100644
--- a/common/lib/xmodule/xmodule/modulestore/__init__.py
+++ b/common/lib/xmodule/xmodule/modulestore/__init__.py
@@ -5,7 +5,7 @@ that are stored in a database an accessible using their Location as an identifie
import re
from collections import namedtuple
-from .exceptions import InvalidLocationError
+from .exceptions import InvalidLocationError, InsufficientSpecificationError
import logging
log = logging.getLogger('mitx.' + 'modulestore')
@@ -38,15 +38,15 @@ class Location(_LocationBase):
'''
__slots__ = ()
- @classmethod
- def clean(cls, value):
+ @staticmethod
+ def clean(value):
"""
Return value, made into a form legal for locations
"""
return re.sub('_+', '_', INVALID_CHARS.sub('_', value))
- @classmethod
- def is_valid(cls, value):
+ @staticmethod
+ def is_valid(value):
'''
Check if the value is a valid location, in any acceptable format.
'''
@@ -56,6 +56,21 @@ class Location(_LocationBase):
return False
return True
+ @staticmethod
+ def ensure_fully_specified(location):
+ '''Make sure location is valid, and fully specified. Raises
+ InvalidLocationError or InsufficientSpecificationError if not.
+
+ returns a Location object corresponding to location.
+ '''
+ loc = Location(location)
+ for key, val in loc.dict().iteritems():
+ if key != 'revision' and val is None:
+ raise InsufficientSpecificationError(location)
+ return loc
+
+
+
def __new__(_cls, loc_or_tag=None, org=None, course=None, category=None,
name=None, revision=None):
"""
@@ -254,25 +269,11 @@ class ModuleStore(object):
'''
raise NotImplementedError
- def path_to_location(self, location, course=None, chapter=None, section=None):
- '''
- Try to find a course/chapter/section[/position] path to this location.
+ def get_parent_locations(self, location):
+ '''Find all locations that are the parents of this location. Needed
+ for path_to_location().
- raise ItemNotFoundError if the location doesn't exist.
-
- If course, chapter, section are not None, restrict search to paths with those
- components as specified.
-
- raise NoPathToItem if the location exists, but isn't accessible via
- a path that matches the course/chapter/section restrictions.
-
- In general, a location may be accessible via many paths. This method may
- return any valid path.
-
- Return a tuple (course, chapter, section, position).
-
- If the section a sequence, position should be the position of this location
- in that sequence. Otherwise, position should be None.
+ returns an iterable of things that can be passed to Location.
'''
raise NotImplementedError
diff --git a/common/lib/xmodule/xmodule/modulestore/mongo.py b/common/lib/xmodule/xmodule/modulestore/mongo.py
index df4e20f3a7..061e2aafe9 100644
--- a/common/lib/xmodule/xmodule/modulestore/mongo.py
+++ b/common/lib/xmodule/xmodule/modulestore/mongo.py
@@ -9,11 +9,10 @@ from importlib import import_module
from xmodule.errorhandlers import strict_error_handler
from xmodule.x_module import XModuleDescriptor
from xmodule.mako_module import MakoDescriptorSystem
-from xmodule.course_module import CourseDescriptor
from mitxmako.shortcuts import render_to_string
from . import ModuleStore, Location
-from .exceptions import (ItemNotFoundError, InsufficientSpecificationError,
+from .exceptions import (ItemNotFoundError,
NoPathToItem, DuplicateItemError)
# TODO (cpennington): This code currently operates under the assumption that
@@ -172,12 +171,17 @@ class MongoModuleStore(ModuleStore):
return self.get_items(course_filter)
def _find_one(self, location):
- '''Look for a given location in the collection.
- If revision isn't specified, returns the latest.'''
- return self.collection.find_one(
+ '''Look for a given location in the collection. If revision is not
+ specified, returns the latest. If the item is not present, raise
+ ItemNotFoundError.
+ '''
+ item = self.collection.find_one(
location_to_query(location),
sort=[('revision', pymongo.ASCENDING)],
)
+ if item is None:
+ raise ItemNotFoundError(location)
+ return item
def get_item(self, location, depth=0):
"""
@@ -197,14 +201,8 @@ class MongoModuleStore(ModuleStore):
calls to get_children() to cache. None indicates to cache all descendents.
"""
-
- for key, val in Location(location).dict().iteritems():
- if key != 'revision' and val is None:
- raise InsufficientSpecificationError(location)
-
+ location = Location.ensure_fully_specified(location)
item = self._find_one(location)
- if item is None:
- raise ItemNotFoundError(location)
return self._load_items([item], depth)[0]
def get_items(self, location, depth=0):
@@ -282,96 +280,20 @@ class MongoModuleStore(ModuleStore):
)
def get_parent_locations(self, location):
- '''Find all locations that are the parents of this location.
- Mostly intended for use in path_to_location, but exposed for testing
- and possible other usefulness.
+ '''Find all locations that are the parents of this location. Needed
+ for path_to_location().
- returns an iterable of things that can be passed to Location.
+ If there is no data at location in this modulestore, raise
+ ItemNotFoundError.
+
+ returns an iterable of things that can be passed to Location. This may
+ be empty if there are no parents.
'''
- location = Location(location)
+ location = Location.ensure_fully_specified(location)
+ # Check that it's actually in this modulestore.
+ item = self._find_one(location)
+ # now get the parents
items = self.collection.find({'definition.children': str(location)},
{'_id': True})
return [i['_id'] for i in items]
- def path_to_location(self, location, course_name=None):
- '''
- Try to find a course_id/chapter/section[/position] path to this location.
- The courseware insists that the first level in the course is chapter,
- but any kind of module can be a "section".
-
- location: something that can be passed to Location
- course_name: [optional]. If not None, restrict search to paths
- in that course.
-
- raise ItemNotFoundError if the location doesn't exist.
-
- raise NoPathToItem if the location exists, but isn't accessible via
- a chapter/section path in the course(s) being searched.
-
- Return a tuple (course_id, chapter, section, position) suitable for the
- courseware index view.
-
- A location may be accessible via many paths. This method may
- return any valid path.
-
- If the section is a sequence, position will be the position
- of this location in that sequence. Otherwise, position will
- be None. TODO (vshnayder): Not true yet.
- '''
- # Check that location is present at all
- if self._find_one(location) is None:
- raise ItemNotFoundError(location)
-
- def flatten(xs):
- '''Convert lisp-style (a, (b, (c, ()))) lists into a python list.
- Not a general flatten function. '''
- p = []
- while xs != ():
- p.append(xs[0])
- xs = xs[1]
- return p
-
- def find_path_to_course(location, course_name=None):
- '''Find a path up the location graph to a node with the
- specified category. If no path exists, return None. If a
- path exists, return it as a list with target location
- first, and the starting location last.
- '''
- # Standard DFS
-
- # To keep track of where we came from, the work queue has
- # tuples (location, path-so-far). To avoid lots of
- # copying, the path-so-far is stored as a lisp-style
- # list--nested hd::tl tuples, and flattened at the end.
- queue = [(location, ())]
- while len(queue) > 0:
- (loc, path) = queue.pop() # Takes from the end
- loc = Location(loc)
- # print 'Processing loc={0}, path={1}'.format(loc, path)
- if loc.category == "course":
- if course_name is None or course_name == loc.name:
- # Found it!
- path = (loc, path)
- return flatten(path)
-
- # otherwise, add parent locations at the end
- newpath = (loc, path)
- parents = self.get_parent_locations(loc)
- queue.extend(zip(parents, repeat(newpath)))
-
- # If we're here, there is no path
- return None
-
- path = find_path_to_course(location, course_name)
- if path is None:
- raise(NoPathToItem(location))
-
- n = len(path)
- course_id = CourseDescriptor.location_to_id(path[0])
- chapter = path[1].name if n > 1 else None
- section = path[2].name if n > 2 else None
-
- # TODO (vshnayder): not handling position at all yet...
- position = None
-
- return (course_id, chapter, section, position)
diff --git a/common/lib/xmodule/xmodule/modulestore/search.py b/common/lib/xmodule/xmodule/modulestore/search.py
new file mode 100644
index 0000000000..a383b3f8ec
--- /dev/null
+++ b/common/lib/xmodule/xmodule/modulestore/search.py
@@ -0,0 +1,96 @@
+from itertools import repeat
+
+from xmodule.course_module import CourseDescriptor
+
+from .exceptions import (ItemNotFoundError, NoPathToItem)
+from . import ModuleStore, Location
+
+
+def path_to_location(modulestore, location, course_name=None):
+ '''
+ Try to find a course_id/chapter/section[/position] path to location in
+ modulestore. The courseware insists that the first level in the course is
+ chapter, but any kind of module can be a "section".
+
+ location: something that can be passed to Location
+ course_name: [optional]. If not None, restrict search to paths
+ in that course.
+
+ raise ItemNotFoundError if the location doesn't exist.
+
+ raise NoPathToItem if the location exists, but isn't accessible via
+ a chapter/section path in the course(s) being searched.
+
+ Return a tuple (course_id, chapter, section, position) suitable for the
+ courseware index view.
+
+ A location may be accessible via many paths. This method may
+ return any valid path.
+
+ If the section is a sequence, position will be the position
+ of this location in that sequence. Otherwise, position will
+ be None. TODO (vshnayder): Not true yet.
+ '''
+
+ def flatten(xs):
+ '''Convert lisp-style (a, (b, (c, ()))) list into a python list.
+ Not a general flatten function. '''
+ p = []
+ while xs != ():
+ p.append(xs[0])
+ xs = xs[1]
+ return p
+
+ def find_path_to_course(location, course_name=None):
+ '''Find a path up the location graph to a node with the
+ specified category.
+
+ If no path exists, return None.
+
+ If a path exists, return it as a list with target location first, and
+ the starting location last.
+ '''
+ # Standard DFS
+
+ # To keep track of where we came from, the work queue has
+ # tuples (location, path-so-far). To avoid lots of
+ # copying, the path-so-far is stored as a lisp-style
+ # list--nested hd::tl tuples, and flattened at the end.
+ queue = [(location, ())]
+ while len(queue) > 0:
+ (loc, path) = queue.pop() # Takes from the end
+ loc = Location(loc)
+
+ # get_parent_locations should raise ItemNotFoundError if location
+ # isn't found so we don't have to do it explicitly. Call this
+ # first to make sure the location is there (even if it's a course, and
+ # we would otherwise immediately exit).
+ parents = modulestore.get_parent_locations(loc)
+
+ # print 'Processing loc={0}, path={1}'.format(loc, path)
+ if loc.category == "course":
+ if course_name is None or course_name == loc.name:
+ # Found it!
+ path = (loc, path)
+ return flatten(path)
+
+ # otherwise, add parent locations at the end
+ newpath = (loc, path)
+ queue.extend(zip(parents, repeat(newpath)))
+
+ # If we're here, there is no path
+ return None
+
+ path = find_path_to_course(location, course_name)
+ if path is None:
+ raise(NoPathToItem(location))
+
+ n = len(path)
+ course_id = CourseDescriptor.location_to_id(path[0])
+ chapter = path[1].name if n > 1 else None
+ section = path[2].name if n > 2 else None
+
+ # TODO (vshnayder): not handling position at all yet...
+ position = None
+
+ return (course_id, chapter, section, position)
diff --git a/common/lib/xmodule/xmodule/modulestore/tests/test_mongo.py b/common/lib/xmodule/xmodule/modulestore/tests/test_mongo.py
index cb2bc6e20c..24f0441ee0 100644
--- a/common/lib/xmodule/xmodule/modulestore/tests/test_mongo.py
+++ b/common/lib/xmodule/xmodule/modulestore/tests/test_mongo.py
@@ -8,6 +8,7 @@ from xmodule.modulestore import Location
from xmodule.modulestore.exceptions import InvalidLocationError, ItemNotFoundError, NoPathToItem
from xmodule.modulestore.mongo import MongoModuleStore
from xmodule.modulestore.xml_importer import import_from_xml
+from xmodule.modulestore.search import path_to_location
# from ~/mitx_all/mitx/common/lib/xmodule/xmodule/modulestore/tests/
# to ~/mitx_all/mitx/common/test
@@ -28,7 +29,7 @@ DEFAULT_CLASS = 'xmodule.raw_module.RawDescriptor'
class TestMongoModuleStore(object):
-
+ '''Tests!'''
@classmethod
def setupClass(cls):
cls.connection = pymongo.connection.Connection(HOST, PORT)
@@ -67,7 +68,7 @@ class TestMongoModuleStore(object):
def test_init(self):
'''Make sure the db loads, and print all the locations in the db.
- Call this directly from failing tests to see what's loaded'''
+ Call this directly from failing tests to see what is loaded'''
ids = list(self.connection[DB][COLLECTION].find({}, {'_id': True}))
pprint([Location(i['_id']).url() for i in ids])
@@ -93,8 +94,6 @@ class TestMongoModuleStore(object):
self.store.get_item("i4x://edX/toy/video/Welcome"),
None)
-
-
def test_find_one(self):
assert_not_equals(
self.store._find_one(Location("i4x://edX/toy/course/2012_Fall")),
@@ -117,13 +116,13 @@ class TestMongoModuleStore(object):
("edX/toy/2012_Fall", "Overview", "Toy_Videos", None)),
)
for location, expected in should_work:
- assert_equals(self.store.path_to_location(location), expected)
+ assert_equals(path_to_location(self.store, location), expected)
not_found = (
- "i4x://edX/toy/video/WelcomeX",
+ "i4x://edX/toy/video/WelcomeX", "i4x://edX/toy/course/NotHome"
)
for location in not_found:
- assert_raises(ItemNotFoundError, self.store.path_to_location, location)
+ assert_raises(ItemNotFoundError, path_to_location, self.store, location)
# Since our test files are valid, there shouldn't be any
# elements with no path to them. But we can look for them in
@@ -132,5 +131,5 @@ class TestMongoModuleStore(object):
"i4x://edX/simple/video/Lost_Video",
)
for location in no_path:
- assert_raises(NoPathToItem, self.store.path_to_location, location, "toy")
+ assert_raises(NoPathToItem, path_to_location, self.store, location, "toy")
diff --git a/common/lib/xmodule/xmodule/xml_module.py b/common/lib/xmodule/xmodule/xml_module.py
index 188df21130..de62bc5d38 100644
--- a/common/lib/xmodule/xmodule/xml_module.py
+++ b/common/lib/xmodule/xmodule/xml_module.py
@@ -163,7 +163,7 @@ class XmlDescriptor(XModuleDescriptor):
return etree.parse(file_object).getroot()
@classmethod
- def definition_loader(cls, xml_object, system, location):
+ def load_definition(cls, xml_object, system, location):
'''Load a descriptor definition from the specified xml_object.
Subclasses should not need to override this except in special
cases (e.g. html module)'''
@@ -225,7 +225,7 @@ class XmlDescriptor(XModuleDescriptor):
slug = xml_object.get('url_name', xml_object.get('slug'))
location = Location('i4x', org, course, xml_object.tag, slug)
- def metadata_loader():
+ def load_metadata():
metadata = {}
for attr in cls.metadata_attributes:
val = xml_object.get(attr)
@@ -237,8 +237,8 @@ class XmlDescriptor(XModuleDescriptor):
metadata[attr_map.metadata_key] = attr_map.to_metadata(val)
return metadata
- definition = cls.definition_loader(xml_object, system, location)
- metadata = metadata_loader()
+ definition = cls.load_definition(xml_object, system, location)
+ metadata = load_metadata()
# VS[compat] -- just have the url_name lookup once translation is done
slug = xml_object.get('url_name', xml_object.get('slug'))
return cls(
diff --git a/lms/djangoapps/courseware/views.py b/lms/djangoapps/courseware/views.py
index 7281ab01ad..a52f715efd 100644
--- a/lms/djangoapps/courseware/views.py
+++ b/lms/djangoapps/courseware/views.py
@@ -20,6 +20,7 @@ from module_render import toc_for_course, get_module, get_section
from models import StudentModuleCache
from student.models import UserProfile
from xmodule.modulestore import Location
+from xmodule.modulestore.search import path_to_location
from xmodule.modulestore.exceptions import InvalidLocationError, ItemNotFoundError, NoPathToItem
from xmodule.modulestore.django import modulestore
from xmodule.course_module import CourseDescriptor
@@ -233,7 +234,7 @@ def jump_to(request, location):
# Complain if there's not data for this location
try:
- (course_id, chapter, section, position) = modulestore().path_to_location(location)
+ (course_id, chapter, section, position) = path_to_location(modulestore(), location)
except ItemNotFoundError:
raise Http404("No data at this location: {0}".format(location))
except NoPathToItem: