diff --git a/common/lib/xmodule/xmodule/course_module.py b/common/lib/xmodule/xmodule/course_module.py index 1eda54ea32..dfac1ac9c6 100644 --- a/common/lib/xmodule/xmodule/course_module.py +++ b/common/lib/xmodule/xmodule/course_module.py @@ -28,17 +28,30 @@ class CourseDescriptor(SequenceDescriptor): def has_started(self): return time.gmtime() > self.start - @classmethod - def id_to_location(cls, course_id): + @staticmethod + def id_to_location(course_id): '''Convert the given course_id (org/course/name) to a location object. Throws ValueError if course_id is of the wrong format. ''' org, course, name = course_id.split('/') return Location('i4x', org, course, 'course', name) + @staticmethod + def location_to_id(location): + '''Convert a location of a course to a course_id. If location category + is not "course", raise a ValueError. + + location: something that can be passed to Location + ''' + loc = Location(location) + if loc.category != "course": + raise ValueError("{0} is not a course location".format(loc)) + return "/".join([loc.org, loc.course, loc.name]) + + @property def id(self): - return "/".join([self.location.org, self.location.course, self.location.name]) + return self.location_to_id(self.location) @property def start_date_text(self): diff --git a/common/lib/xmodule/xmodule/modulestore/mongo.py b/common/lib/xmodule/xmodule/modulestore/mongo.py index d2b68d03ef..e706f312f8 100644 --- a/common/lib/xmodule/xmodule/modulestore/mongo.py +++ b/common/lib/xmodule/xmodule/modulestore/mongo.py @@ -9,16 +9,18 @@ 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, InsufficientSpecificationError, NoPathToItem # TODO (cpennington): This code currently operates under the assumption that # there is only one revision for each item. Once we start versioning inside the CMS, # that assumption will have to change + class CachingDescriptorSystem(MakoDescriptorSystem): """ A system that has a cache of module json that it will use to load modules @@ -167,6 +169,14 @@ class MongoModuleStore(ModuleStore): course_filter = Location("i4x", category="course") 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( + location_to_query(location), + sort=[('revision', pymongo.ASCENDING)], + ) + def get_item(self, location, depth=0): """ Returns an XModuleDescriptor instance for the item at location. @@ -190,10 +200,7 @@ class MongoModuleStore(ModuleStore): if key != 'revision' and val is None: raise InsufficientSpecificationError(location) - item = self.collection.find_one( - location_to_query(location), - sort=[('revision', pymongo.ASCENDING)], - ) + item = self._find_one(location) if item is None: raise ItemNotFoundError(location) return self._load_items([item], depth)[0] @@ -265,24 +272,101 @@ class MongoModuleStore(ModuleStore): {'$set': {'metadata': metadata}} ) - def path_to_location(self, location, course=None): + 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. + + returns an iterable of things that can be passed to Location. ''' - Try to find a course/chapter/section[/position] path to this location. + location = Location(location) + 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. - If course is not None, restrict search to paths in that course. - raise NoPathToItem if the location exists, but isn't accessible via a chapter/section path in the course(s) being searched. - In general, a location may be accessible via many paths. This method may + 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. - 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. + 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. ''' - raise NotImplementedError + # 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/tests/test_mongo.py b/common/lib/xmodule/xmodule/modulestore/tests/test_mongo.py index 493b8a385d..cb2bc6e20c 100644 --- a/common/lib/xmodule/xmodule/modulestore/tests/test_mongo.py +++ b/common/lib/xmodule/xmodule/modulestore/tests/test_mongo.py @@ -2,9 +2,10 @@ import pymongo from nose.tools import assert_equals, assert_raises, assert_not_equals, with_setup from path import path +from pprint import pprint from xmodule.modulestore import Location -from xmodule.modulestore.exceptions import InvalidLocationError, ItemNotFoundError +from xmodule.modulestore.exceptions import InvalidLocationError, ItemNotFoundError, NoPathToItem from xmodule.modulestore.mongo import MongoModuleStore from xmodule.modulestore.xml_importer import import_from_xml @@ -33,28 +34,44 @@ class TestMongoModuleStore(object): cls.connection = pymongo.connection.Connection(HOST, PORT) cls.connection.drop_database(DB) + # NOTE: Creating a single db for all the tests to save time. This + # is ok only as long as none of the tests modify the db. + # If (when!) that changes, need to either reload the db, or load + # once and copy over to a tmp db for each test. + cls.store = cls.initdb() + @classmethod def teardownClass(cls): pass - def setUp(self): + @staticmethod + def initdb(): # connect to the db - self.store = MongoModuleStore(HOST, DB, COLLECTION, FS_ROOT, default_class=DEFAULT_CLASS) + store = MongoModuleStore(HOST, DB, COLLECTION, FS_ROOT, default_class=DEFAULT_CLASS) # Explicitly list the courses to load (don't want the big one) courses = ['toy', 'simple'] - import_from_xml(self.store, DATA_DIR, courses) - self.connection = TestMongoModuleStore.connection - - def tearDown(self): + import_from_xml(store, DATA_DIR, courses) + return store + + @staticmethod + def destroy_db(connection): # Destroy the test db. - self.connection.drop_database(DB) - self.store = None + connection.drop_database(DB) + + def setUp(self): + # make a copy for convenience + self.connection = TestMongoModuleStore.connection + + def tearDown(self): + pass def test_init(self): - '''Just make sure the db loads''' + '''Make sure the db loads, and print all the locations in the db. + Call this directly from failing tests to see what's loaded''' ids = list(self.connection[DB][COLLECTION].find({}, {'_id': True})) - print len(ids) - + + pprint([Location(i['_id']).url() for i in ids]) + def test_get_courses(self): '''Make sure the course objects loaded properly''' courses = self.store.get_courses() @@ -63,10 +80,41 @@ class TestMongoModuleStore(object): assert_equals(courses[0].id, 'edX/simple/2012_Fall') assert_equals(courses[1].id, 'edX/toy/2012_Fall') - def Xtest_path_to_location(self): + def test_loads(self): + assert_not_equals( + self.store.get_item("i4x://edX/toy/course/2012_Fall"), + None) + + assert_not_equals( + self.store.get_item("i4x://edX/simple/course/2012_Fall"), + None) + + assert_not_equals( + 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")), + None) + + assert_not_equals( + self.store._find_one(Location("i4x://edX/simple/course/2012_Fall")), + None) + + assert_not_equals( + self.store._find_one(Location("i4x://edX/toy/video/Welcome")), + None) + + def test_path_to_location(self): '''Make sure that path_to_location works''' should_work = ( - ("i4x://edX/toy/video/Welcome", ("toy", "Overview", None, None)), + ("i4x://edX/toy/video/Welcome", + ("edX/toy/2012_Fall", "Overview", "Welcome", None)), + ("i4x://edX/toy/html/toylab", + ("edX/toy/2012_Fall", "Overview", "Toy_Videos", None)), ) for location, expected in should_work: assert_equals(self.store.path_to_location(location), expected) @@ -76,10 +124,13 @@ class TestMongoModuleStore(object): ) for location in not_found: assert_raises(ItemNotFoundError, self.store.path_to_location, 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 + # another course. no_path = ( - "i4x://edX/toy/video/Lost_Video", + "i4x://edX/simple/video/Lost_Video", ) - for location in not_found: - assert_raises(ItemNotFoundError, self.store.path_to_location, location) - + for location in no_path: + assert_raises(NoPathToItem, self.store.path_to_location, location, "toy") + diff --git a/common/test/data/simple/course.xml b/common/test/data/simple/course.xml index 59a0a0c5bc..b92158cdb7 100644 --- a/common/test/data/simple/course.xml +++ b/common/test/data/simple/course.xml @@ -18,7 +18,7 @@ +