diff --git a/common/lib/xmodule/xmodule/modulestore/__init__.py b/common/lib/xmodule/xmodule/modulestore/__init__.py index 5b94add68f..f86a6e9600 100644 --- a/common/lib/xmodule/xmodule/modulestore/__init__.py +++ b/common/lib/xmodule/xmodule/modulestore/__init__.py @@ -345,9 +345,9 @@ class ModuleStore(object): ''' raise NotImplementedError - def get_parent_locations(self, location): - '''Find all locations that are the parents of this location. Needed - for path_to_location(). + def get_parent_locations(self, location, course_id): + '''Find all locations that are the parents of this location in this + course. Needed for path_to_location(). returns an iterable of things that can be passed to Location. ''' diff --git a/common/lib/xmodule/xmodule/modulestore/mongo.py b/common/lib/xmodule/xmodule/modulestore/mongo.py index baa4e7870c..4c7ef3c050 100644 --- a/common/lib/xmodule/xmodule/modulestore/mongo.py +++ b/common/lib/xmodule/xmodule/modulestore/mongo.py @@ -309,9 +309,9 @@ class MongoModuleStore(ModuleStoreBase): self._update_single_item(location, {'metadata': metadata}) - def get_parent_locations(self, location): - '''Find all locations that are the parents of this location. Needed - for path_to_location(). + def get_parent_locations(self, location, course_id): + '''Find all locations that are the parents of this location in this + course. Needed for path_to_location(). If there is no data at location in this modulestore, raise ItemNotFoundError. diff --git a/common/lib/xmodule/xmodule/modulestore/search.py b/common/lib/xmodule/xmodule/modulestore/search.py index f9901e8bfe..4a5ece6854 100644 --- a/common/lib/xmodule/xmodule/modulestore/search.py +++ b/common/lib/xmodule/xmodule/modulestore/search.py @@ -64,7 +64,7 @@ def path_to_location(modulestore, course_id, 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) + parents = modulestore.get_parent_locations(loc, course_id) # print 'Processing loc={0}, path={1}'.format(loc, path) if loc.category == "course": diff --git a/common/lib/xmodule/xmodule/modulestore/tests/test_modulestore.py b/common/lib/xmodule/xmodule/modulestore/tests/test_modulestore.py index c1d1d50a53..64816581ce 100644 --- a/common/lib/xmodule/xmodule/modulestore/tests/test_modulestore.py +++ b/common/lib/xmodule/xmodule/modulestore/tests/test_modulestore.py @@ -23,12 +23,3 @@ def check_path_to_location(modulestore): for location in not_found: assert_raises(ItemNotFoundError, path_to_location, modulestore, course_id, 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/simple/video/Lost_Video", - ) - for location in no_path: - assert_raises(NoPathToItem, path_to_location, modulestore, course_id, location) - diff --git a/common/lib/xmodule/xmodule/modulestore/xml.py b/common/lib/xmodule/xmodule/modulestore/xml.py index 6b3ff9bff4..04f3a94d1b 100644 --- a/common/lib/xmodule/xmodule/modulestore/xml.py +++ b/common/lib/xmodule/xmodule/modulestore/xml.py @@ -275,14 +275,16 @@ class XMLModuleStore(ModuleStoreBase): class_ = getattr(import_module(module_path), class_name) self.default_class = class_ - self.parent_tracker = ParentTracker() + self.parent_trackers = defaultdict(ParentTracker) # If we are specifically asked for missing courses, that should # be an error. If we are asked for "all" courses, find the ones - # that have a course.xml + # that have a course.xml. We sort the dirs in alpha order so we always + # read things in the same order (OS differences in load order have + # bitten us in the past.) if course_dirs is None: - course_dirs = [d for d in os.listdir(self.data_dir) if - os.path.exists(self.data_dir / d / "course.xml")] + course_dirs = sorted([d for d in os.listdir(self.data_dir) if + os.path.exists(self.data_dir / d / "course.xml")]) for course_dir in course_dirs: self.try_load_course(course_dir) @@ -307,7 +309,7 @@ class XMLModuleStore(ModuleStoreBase): if course_descriptor is not None: self.courses[course_dir] = course_descriptor self._location_errors[course_descriptor.location] = errorlog - self.parent_tracker.make_known(course_descriptor.location) + self.parent_trackers[course_descriptor.id].make_known(course_descriptor.location) else: # Didn't load course. Instead, save the errors elsewhere. self.errored_courses[course_dir] = errorlog @@ -432,7 +434,7 @@ class XMLModuleStore(ModuleStoreBase): course_dir, policy, tracker, - self.parent_tracker, + self.parent_trackers[course_id], self.load_error_modules, ) @@ -541,9 +543,9 @@ class XMLModuleStore(ModuleStoreBase): """ raise NotImplementedError("XMLModuleStores are read-only") - def get_parent_locations(self, location): - '''Find all locations that are the parents of this location. Needed - for path_to_location(). + def get_parent_locations(self, location, course_id): + '''Find all locations that are the parents of this location in this + course. Needed for path_to_location(). If there is no data at location in this modulestore, raise ItemNotFoundError. @@ -552,7 +554,7 @@ class XMLModuleStore(ModuleStoreBase): be empty if there are no parents. ''' location = Location.ensure_fully_specified(location) - if not self.parent_tracker.is_known(location): - raise ItemNotFoundError(location) + if not self.parent_trackers[course_id].is_known(location): + raise ItemNotFoundError("{0} not in {1}".format(location, course_id)) - return self.parent_tracker.parents(location) + return self.parent_trackers[course_id].parents(location)