location mapper maturation

richer exceptions, add_map return course_id, fix a bug, filter out unwanted kwargs in init

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):

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

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'),

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(