diff --git a/common/lib/xmodule/xmodule/modulestore/exceptions.py b/common/lib/xmodule/xmodule/modulestore/exceptions.py index 508599b677..411078d821 100644 --- a/common/lib/xmodule/xmodule/modulestore/exceptions.py +++ b/common/lib/xmodule/xmodule/modulestore/exceptions.py @@ -28,7 +28,14 @@ class NoPathToItem(Exception): class DuplicateItemError(Exception): - pass + """ + Attempted to create an item which already exists. + """ + def __init__(self, element_id, store=None, collection=None): + super(DuplicateItemError, self).__init__() + self.element_id = element_id + self.store = store + self.collection = collection class VersionConflictError(Exception): diff --git a/common/lib/xmodule/xmodule/modulestore/loc_mapper_store.py b/common/lib/xmodule/xmodule/modulestore/loc_mapper_store.py index 5967ee3d9d..4ab3a8106d 100644 --- a/common/lib/xmodule/xmodule/modulestore/loc_mapper_store.py +++ b/common/lib/xmodule/xmodule/modulestore/loc_mapper_store.py @@ -28,10 +28,16 @@ class LocMapperStore(object): # C0103: varnames and attrs must be >= 3 chars, but db defined by long time usage # pylint: disable = C0103 - def __init__(self, host, db, collection, port=27017, user=None, password=None, **kwargs): + def __init__(self, host, db, collection, port=27017, user=None, password=None, + **kwargs): ''' Constructor ''' + # get rid of unwanted args + kwargs.pop('default_class', None) + kwargs.pop('fs_root', None) + kwargs.pop('xblock_mixins', None) + kwargs.pop('render_template', None) self.db = pymongo.database.Database( pymongo.MongoClient( host=host, @@ -100,6 +106,7 @@ class LocMapperStore(object): 'prod_branch': prod_branch, 'block_map': block_map or {}, }) + return course_id def translate_location(self, old_style_course_id, location, published=True, add_entry_if_missing=True): """ @@ -150,7 +157,7 @@ class LocMapperStore(object): if add_entry_if_missing: usage_id = self._add_to_block_map(location, location_id, entry['block_map']) else: - raise ItemNotFoundError() + raise ItemNotFoundError(location) elif isinstance(usage_id, dict): # name is not unique, look through for the right category if location.category in usage_id: @@ -244,7 +251,7 @@ class LocMapperStore(object): if usage_id is None: usage_id = map_entry['block_map'][location.name][location.category] elif usage_id != map_entry['block_map'][location.name][location.category]: - raise DuplicateItemError() + raise DuplicateItemError(usage_id, self, 'location_map') computed_usage_id = usage_id @@ -257,7 +264,7 @@ class LocMapperStore(object): alt_usage_id = self._verify_uniqueness(computed_usage_id, map_entry['block_map']) if alt_usage_id != computed_usage_id: if usage_id is not None: - raise DuplicateItemError() + raise DuplicateItemError(usage_id, self, 'location_map') else: # revise already set ones and add to remaining ones computed_usage_id = self.update_block_location_translator( @@ -301,7 +308,7 @@ class LocMapperStore(object): usage_id = self.update_block_location_translator(location, alt_usage_id, old_course_id, True) return usage_id else: - raise DuplicateItemError() + raise DuplicateItemError(usage_id, self, 'location_map') if location.category in map_entry['block_map'].setdefault(location.name, {}): map_entry['block_map'][location.name][location.category] = usage_id @@ -335,6 +342,8 @@ class LocMapperStore(object): # the block ids will likely be out of sync and collide from an id perspective. HOWEVER, # if there are few == org/course roots or their content is unrelated, this will work well. usage_id = self._verify_uniqueness(location.category + location.name[:3], block_map) + else: + usage_id = location.name block_map.setdefault(location.name, {})[location.category] = usage_id self.location_map.update(location_id, {'$set': {'block_map': block_map}}) return usage_id diff --git a/common/lib/xmodule/xmodule/modulestore/split_mongo/caching_descriptor_system.py b/common/lib/xmodule/xmodule/modulestore/split_mongo/caching_descriptor_system.py index 020ebdbbfe..f297972367 100644 --- a/common/lib/xmodule/xmodule/modulestore/split_mongo/caching_descriptor_system.py +++ b/common/lib/xmodule/xmodule/modulestore/split_mongo/caching_descriptor_system.py @@ -30,13 +30,11 @@ class CachingDescriptorSystem(MakoDescriptorSystem): module_data: a dict mapping Location -> json that was cached from the underlying modulestore """ - # TODO find all references to resources_fs and make handle None super(CachingDescriptorSystem, self).__init__(load_item=self._load_item, **kwargs) self.modulestore = modulestore self.course_entry = course_entry self.lazy = lazy self.module_data = module_data - # TODO see if self.course_id is needed: is already in course_entry but could be > 1 value # Compute inheritance modulestore.inherit_settings( course_entry.get('blocks', {}), @@ -60,7 +58,7 @@ class CachingDescriptorSystem(MakoDescriptorSystem): self.modulestore.cache_items(self, [usage_id], lazy=self.lazy) json_data = self.module_data.get(usage_id) if json_data is None: - raise ItemNotFoundError + raise ItemNotFoundError(usage_id) class_ = XModuleDescriptor.load_class( json_data.get('category'), diff --git a/common/lib/xmodule/xmodule/modulestore/tests/test_location_mapper.py b/common/lib/xmodule/xmodule/modulestore/tests/test_location_mapper.py index 599bd6da5e..abe41eaaed 100644 --- a/common/lib/xmodule/xmodule/modulestore/tests/test_location_mapper.py +++ b/common/lib/xmodule/xmodule/modulestore/tests/test_location_mapper.py @@ -192,6 +192,14 @@ class TestLocationMapper(unittest.TestCase): add_entry_if_missing=True ) self.assertEqual(prob_locator.course_id, new_style_course_id) + # create an entry w/o a guid name + other_location = Location('i4x', org, course, 'chapter', 'intro') + locator = loc_mapper().translate_location( + old_style_course_id, + other_location, + add_entry_if_missing=True + ) + self.assertEqual(locator.usage_id, 'intro') # add a distractor course loc_mapper().create_map_entry(