Change new BlockData object from a dict to an object with specific

attributes that are serialized/de-serialized to/from MongoDB.

Change access of attributes from dict keys.

Add EditInfo object to encapsulate editing info for block data.

common/lib/xmodule/xmodule/modulestore/__init__.py

@@ -37,6 +37,7 @@ log = logging.getLogger('edx.modulestore')
 new_contract('CourseKey', CourseKey)
 new_contract('AssetKey', AssetKey)
 new_contract('AssetMetadata', AssetMetadata)
+new_contract('XBlock', XBlock)
 
 
 class ModuleStoreEnum(object):
@@ -276,6 +277,122 @@ class BulkOperationsMixin(object):
         return self._get_bulk_ops_record(course_key, ignore_case).active
 
 
+class EditInfo(object):
+    """
+    Encapsulates the editing info of a block.
+    """
+    def __init__(self, **kwargs):
+        self.from_storable(kwargs)
+
+        # For details, see caching_descriptor_system.py get_subtree_edited_by/on.
+        self._subtree_edited_on = kwargs.get('_subtree_edited_on', None)
+        self._subtree_edited_by = kwargs.get('_subtree_edited_by', None)
+
+    def to_storable(self):
+        """
+        Serialize to a Mongo-storable format.
+        """
+        return {
+            'previous_version': self.previous_version,
+            'update_version': self.update_version,
+            'source_version': self.source_version,
+            'edited_on': self.edited_on,
+            'edited_by': self.edited_by,
+            'original_usage': self.original_usage,
+            'original_usage_version': self.original_usage_version,
+        }
+
+    def from_storable(self, edit_info):
+        """
+        De-serialize from Mongo-storable format to an object.
+        """
+        # Guid for the structure which previously changed this XBlock.
+        # (Will be the previous value of 'update_version'.)
+        self.previous_version = edit_info.get('previous_version', None)
+
+        # Guid for the structure where this XBlock got its current field values.
+        # May point to a structure not in this structure's history (e.g., to a draft
+        # branch from which this version was published).
+        self.update_version = edit_info.get('update_version', None)
+
+        self.source_version = edit_info.get('source_version', None)
+
+        # Datetime when this XBlock's fields last changed.
+        self.edited_on = edit_info.get('edited_on', None)
+        # User ID which changed this XBlock last.
+        self.edited_by = edit_info.get('edited_by', None)
+
+        self.original_usage = edit_info.get('original_usage', None)
+        self.original_usage_version = edit_info.get('original_usage_version', None)
+
+    def __str__(self):
+        return ("EditInfo(previous_version={0.previous_version}, "
+                "update_version={0.update_version}, "
+                "source_version={0.source_version}, "
+                "edited_on={0.edited_on}, "
+                "edited_by={0.edited_by}, "
+                "original_usage={0.original_usage}, "
+                "original_usage_version={0.original_usage_version}, "
+                "_subtree_edited_on={0._subtree_edited_on}, "
+                "_subtree_edited_by={0._subtree_edited_by})").format(self)
+
+
+class BlockData(object):
+    """
+    Wrap the block data in an object instead of using a straight Python dictionary.
+    Allows the storing of meta-information about a structure that doesn't persist along with
+    the structure itself.
+    """
+    def __init__(self, **kwargs):
+        # Has the definition been loaded?
+        self.definition_loaded = False
+        self.from_storable(kwargs)
+
+    def to_storable(self):
+        """
+        Serialize to a Mongo-storable format.
+        """
+        return {
+            'fields': self.fields,
+            'block_type': self.block_type,
+            'definition': self.definition,
+            'defaults': self.defaults,
+            'edit_info': self.edit_info.to_storable()
+        }
+
+    def from_storable(self, block_data):
+        """
+        De-serialize from Mongo-storable format to an object.
+        """
+        # Contains the Scope.settings and 'children' field values.
+        # 'children' are stored as a list of (block_type, block_id) pairs.
+        self.fields = block_data.get('fields', {})
+
+        # XBlock type ID.
+        self.block_type = block_data.get('block_type', None)
+
+        # DB id of the record containing the content of this XBlock.
+        self.definition = block_data.get('definition', None)
+
+        # Scope.settings default values copied from a template block (used e.g. when
+        # blocks are copied from a library to a course)
+        self.defaults = block_data.get('defaults', {})
+
+        # EditInfo object containing all versioning/editing data.
+        self.edit_info = EditInfo(**block_data.get('edit_info', {}))
+
+    def __str__(self):
+        return ("BlockData(fields={0.fields}, "
+                "block_type={0.block_type}, "
+                "definition={0.definition}, "
+                "definition_loaded={0.definition_loaded}, "
+                "defaults={0.defaults}, "
+                "edit_info={0.edit_info})").format(self)
+
+
+new_contract('BlockData', BlockData)
+
+
 class IncorrectlySortedList(Exception):
     """
     Thrown when calling find() on a SortedAssetList not sorted by filename.
@@ -615,27 +732,32 @@ class ModuleStoreRead(ModuleStoreAssetBase):
         """
         pass
 
-    def _block_matches(self, fields_or_xblock, qualifiers):
-        '''
+    @contract(block='XBlock | BlockData | dict', qualifiers=dict)
+    def _block_matches(self, block, qualifiers):
+        """
         Return True or False depending on whether the field value (block contents)
-        matches the qualifiers as per get_items. Note, only finds directly set not
-        inherited nor default value matches.
-        For substring matching pass a regex object.
-        for arbitrary function comparison such as date time comparison, pass
-        the function as in start=lambda x: x < datetime.datetime(2014, 1, 1, 0, tzinfo=pytz.UTC)
+        matches the qualifiers as per get_items.
+        NOTE: Method only finds directly set value matches - not inherited nor default value matches.
+        For substring matching:
+            pass a regex object.
+        For arbitrary function comparison such as date time comparison:
+            pass the function as in start=lambda x: x < datetime.datetime(2014, 1, 1, 0, tzinfo=pytz.UTC)
 
         Args:
-            fields_or_xblock (dict or XBlock): either the json blob (from the db or get_explicitly_set_fields)
-                or the xblock.fields() value or the XBlock from which to get those values
-             qualifiers (dict): field: searchvalue pairs.
-        '''
-        if isinstance(fields_or_xblock, XBlock):
-            fields = fields_or_xblock.fields
-            xblock = fields_or_xblock
-            is_xblock = True
+            block (dict, XBlock, or BlockData): either the BlockData (transformed from the db) -or-
+                a dict (from BlockData.fields or get_explicitly_set_fields_by_scope) -or-
+                the xblock.fields() value -or-
+                the XBlock from which to get the 'fields' value.
+             qualifiers (dict): {field: value} search pairs.
+        """
+        if isinstance(block, XBlock):
+            # If an XBlock is passed-in, just match its fields.
+            xblock, fields = (block, block.fields)
+        elif isinstance(block, BlockData):
+            # BlockData is an object - compare its attributes in dict form.
+            xblock, fields = (None, block.__dict__)
         else:
-            fields = fields_or_xblock
-            is_xblock = False
+            xblock, fields = (None, block)
 
         def _is_set_on(key):
             """
@@ -646,8 +768,8 @@ class ModuleStoreRead(ModuleStoreAssetBase):
             if key not in fields:
                 return False, None
             field = fields[key]
-            if is_xblock:
-                return field.is_set_on(fields_or_xblock), getattr(xblock, key)
+            if xblock is not None:
+                return field.is_set_on(block), getattr(xblock, key)
             else:
                 return True, field
 
@@ -660,7 +782,7 @@ class ModuleStoreRead(ModuleStoreAssetBase):
         return True
 
     def _value_matches(self, target, criteria):
-        '''
+        """
         helper for _block_matches: does the target (field value) match the criteria?
 
         If target is a list, do any of the list elements meet the criteria
@@ -668,7 +790,7 @@ class ModuleStoreRead(ModuleStoreAssetBase):
         If the criteria is a function, does invoking it on the target yield something truthy?
         If criteria is a dict {($nin|$in): []}, then do (none|any) of the list elements meet the criteria
         Otherwise, is the target == criteria
-        '''
+        """
         if isinstance(target, list):
             return any(self._value_matches(ele, criteria) for ele in target)
         elif isinstance(criteria, re._pattern_type):  # pylint: disable=protected-access

common/lib/xmodule/xmodule/modulestore/split_mongo/__init__.py

@@ -21,78 +21,3 @@ class BlockKey(namedtuple('BlockKey', 'type id')):
 
 
 CourseEnvelope = namedtuple('CourseEnvelope', 'course_key structure')
-
-
-class BlockData(object):
-    """
-    Wrap the block data in an object instead of using a straight Python dictionary.
-    Allows the storing of meta-information about a structure that doesn't persist along with
-    the structure itself.
-    """
-    @contract(block_dict=dict)
-    def __init__(self, block_dict={}):  # pylint: disable=dangerous-default-value
-        # Has the definition been loaded?
-        self.definition_loaded = False
-        self.from_storable(block_dict)
-
-    def to_storable(self):
-        """
-        Serialize to a Mongo-storable format.
-        """
-        return {
-            'fields': self.fields,
-            'block_type': self.block_type,
-            'definition': self.definition,
-            'defaults': self.defaults,
-            'edit_info': self.edit_info
-        }
-
-    @contract(stored=dict)
-    def from_storable(self, stored):
-        """
-        De-serialize from Mongo-storable format to an object.
-        """
-        self.fields = stored.get('fields', {})
-        self.block_type = stored.get('block_type', None)
-        self.definition = stored.get('definition', None)
-        self.defaults = stored.get('defaults', {})
-        self.edit_info = stored.get('edit_info', {})
-
-    def get(self, key, *args, **kwargs):
-        """
-        Dict-like 'get' method. Raises AttributeError if requesting non-existent attribute and no default.
-        """
-        if len(args) > 0:
-            return getattr(self, key, args[0])
-        elif 'default' in kwargs:
-            return getattr(self, key, kwargs['default'])
-        else:
-            return getattr(self, key)
-
-    def __getitem__(self, key):
-        """
-        Dict-like '__getitem__'.
-        """
-        if not hasattr(self, key):
-            raise KeyError
-        else:
-            return getattr(self, key)
-
-    def __setitem__(self, key, value):
-        setattr(self, key, value)
-
-    def __delitem__(self, key):
-        delattr(self, key)
-
-    def __iter__(self):
-        return self.__dict__.iterkeys()
-
-    def setdefault(self, key, default=None):
-        """
-        Dict-like 'setdefault'.
-        """
-        try:
-            return getattr(self, key)
-        except AttributeError:
-            setattr(self, key, default)
-            return default

common/lib/xmodule/xmodule/modulestore/split_mongo/caching_descriptor_system.py

@@ -5,11 +5,13 @@ from fs.osfs import OSFS
 from lazy import lazy
 from xblock.runtime import KvsFieldData
 from xblock.fields import ScopeIds
+from xblock.core import XBlock
 from opaque_keys.edx.locator import BlockUsageLocator, LocalId, CourseLocator, LibraryLocator, DefinitionLocator
 from xmodule.library_tools import LibraryToolsService
 from xmodule.mako_module import MakoDescriptorSystem
 from xmodule.error_module import ErrorDescriptor
 from xmodule.errortracker import exc_info_to_str
+from xmodule.modulestore import BlockData
 from xmodule.modulestore.edit_info import EditInfoRuntimeMixin
 from xmodule.modulestore.exceptions import ItemNotFoundError
 from xmodule.modulestore.inheritance import inheriting_field_data, InheritanceMixin
@@ -24,7 +26,9 @@ new_contract('BlockUsageLocator', BlockUsageLocator)
 new_contract('CourseLocator', CourseLocator)
 new_contract('LibraryLocator', LibraryLocator)
 new_contract('BlockKey', BlockKey)
+new_contract('BlockData', BlockData)
 new_contract('CourseEnvelope', CourseEnvelope)
+new_contract('XBlock', XBlock)
 
 
 class CachingDescriptorSystem(MakoDescriptorSystem, EditInfoRuntimeMixin):
@@ -79,7 +83,7 @@ class CachingDescriptorSystem(MakoDescriptorSystem, EditInfoRuntimeMixin):
     def _parent_map(self):
         parent_map = {}
         for block_key, block in self.course_entry.structure['blocks'].iteritems():
-            for child in block['fields'].get('children', []):
+            for child in block.fields.get('children', []):
                 parent_map[child] = block_key
         return parent_map
 
@@ -119,7 +123,7 @@ class CachingDescriptorSystem(MakoDescriptorSystem, EditInfoRuntimeMixin):
 
         block_data = self.get_module_data(block_key, course_key)
 
-        class_ = self.load_block_type(block_data.get('block_type'))
+        class_ = self.load_block_type(block_data.block_type)
         block = self.xblock_from_json(class_, course_key, block_key, block_data, course_entry_override, **kwargs)
         self.modulestore.cache_block(course_key, version_guid, block_key, block)
         return block
@@ -164,17 +168,17 @@ class CachingDescriptorSystem(MakoDescriptorSystem, EditInfoRuntimeMixin):
             # most recent retrieval is most likely the right one for next caller (see comment above fn)
             self.course_entry = CourseEnvelope(course_entry_override.course_key, self.course_entry.structure)
 
-        definition_id = block_data.get('definition')
+        definition_id = block_data.definition
 
         # If no usage id is provided, generate an in-memory id
         if block_key is None:
-            block_key = BlockKey(block_data['block_type'], LocalId())
+            block_key = BlockKey(block_data.block_type, LocalId())
 
         convert_fields = lambda field: self.modulestore.convert_references_to_keys(
             course_key, class_, field, self.course_entry.structure['blocks'],
         )
 
-        if definition_id is not None and not block_data['definition_loaded']:
+        if definition_id is not None and not block_data.definition_loaded:
             definition_loader = DefinitionLazyLoader(
                 self.modulestore,
                 course_key,
@@ -195,8 +199,8 @@ class CachingDescriptorSystem(MakoDescriptorSystem, EditInfoRuntimeMixin):
             block_id=block_key.id,
         )
 
-        converted_fields = convert_fields(block_data.get('fields', {}))
-        converted_defaults = convert_fields(block_data.get('defaults', {}))
+        converted_fields = convert_fields(block_data.fields)
+        converted_defaults = convert_fields(block_data.defaults)
         if block_key in self._parent_map:
             parent_key = self._parent_map[block_key]
             parent = course_key.make_usage_key(parent_key.type, parent_key.id)
@@ -221,7 +225,7 @@ class CachingDescriptorSystem(MakoDescriptorSystem, EditInfoRuntimeMixin):
                 ScopeIds(None, block_key.type, definition_id, block_locator),
                 field_data,
             )
-        except Exception:
+        except Exception:  # pylint: disable=broad-except
             log.warning("Failed to load descriptor", exc_info=True)
             return ErrorDescriptor.from_json(
                 block_data,
@@ -233,12 +237,12 @@ class CachingDescriptorSystem(MakoDescriptorSystem, EditInfoRuntimeMixin):
                 error_msg=exc_info_to_str(sys.exc_info())
             )
 
-        edit_info = block_data.get('edit_info', {})
-        module._edited_by = edit_info.get('edited_by')  # pylint: disable=protected-access
-        module._edited_on = edit_info.get('edited_on')  # pylint: disable=protected-access
-        module.previous_version = edit_info.get('previous_version')
-        module.update_version = edit_info.get('update_version')
-        module.source_version = edit_info.get('source_version', None)
+        edit_info = block_data.edit_info
+        module._edited_by = edit_info.edited_by  # pylint: disable=protected-access
+        module._edited_on = edit_info.edited_on  # pylint: disable=protected-access
+        module.previous_version = edit_info.previous_version
+        module.update_version = edit_info.update_version
+        module.source_version = edit_info.source_version
         module.definition_locator = DefinitionLocator(block_key.type, definition_id)
         # decache any pending field settings
         module.save()
@@ -261,31 +265,35 @@ class CachingDescriptorSystem(MakoDescriptorSystem, EditInfoRuntimeMixin):
         """
         return xblock._edited_on
 
+    @contract(xblock='XBlock')
     def get_subtree_edited_by(self, xblock):
         """
         See :class: cms.lib.xblock.runtime.EditInfoRuntimeMixin
         """
+        # pylint: disable=protected-access
         if not hasattr(xblock, '_subtree_edited_by'):
-            json_data = self.module_data[BlockKey.from_usage_key(xblock.location)]
-            if '_subtree_edited_by' not in json_data.setdefault('edit_info', {}):
+            block_data = self.module_data[BlockKey.from_usage_key(xblock.location)]
+            if block_data.edit_info._subtree_edited_by is None:
                 self._compute_subtree_edited_internal(
-                    xblock.location.block_id, json_data, xblock.location.course_key
+                    block_data, xblock.location.course_key
                 )
-            setattr(xblock, '_subtree_edited_by', json_data['edit_info']['_subtree_edited_by'])
+            setattr(xblock, '_subtree_edited_by', block_data.edit_info._subtree_edited_by)
 
         return getattr(xblock, '_subtree_edited_by')
 
+    @contract(xblock='XBlock')
     def get_subtree_edited_on(self, xblock):
         """
         See :class: cms.lib.xblock.runtime.EditInfoRuntimeMixin
         """
+        # pylint: disable=protected-access
         if not hasattr(xblock, '_subtree_edited_on'):
-            json_data = self.module_data[BlockKey.from_usage_key(xblock.location)]
-            if '_subtree_edited_on' not in json_data.setdefault('edit_info', {}):
+            block_data = self.module_data[BlockKey.from_usage_key(xblock.location)]
+            if block_data.edit_info._subtree_edited_on is None:
                 self._compute_subtree_edited_internal(
-                    xblock.location.block_id, json_data, xblock.location.course_key
+                    block_data, xblock.location.course_key
                 )
-            setattr(xblock, '_subtree_edited_on', json_data['edit_info']['_subtree_edited_on'])
+            setattr(xblock, '_subtree_edited_on', block_data.edit_info._subtree_edited_on)
 
         return getattr(xblock, '_subtree_edited_on')
 
@@ -307,20 +315,22 @@ class CachingDescriptorSystem(MakoDescriptorSystem, EditInfoRuntimeMixin):
 
         return getattr(xblock, '_published_on', None)
 
-    def _compute_subtree_edited_internal(self, block_id, json_data, course_key):
+    @contract(block_data='BlockData')
+    def _compute_subtree_edited_internal(self, block_data, course_key):
         """
-        Recurse the subtree finding the max edited_on date and its concomitant edited_by. Cache it
+        Recurse the subtree finding the max edited_on date and its corresponding edited_by. Cache it.
         """
-        max_date = json_data['edit_info']['edited_on']
-        max_by = json_data['edit_info']['edited_by']
+        # pylint: disable=protected-access
+        max_date = block_data.edit_info.edited_on
+        max_date_by = block_data.edit_info.edited_by
 
-        for child in json_data.get('fields', {}).get('children', []):
+        for child in block_data.fields.get('children', []):
             child_data = self.get_module_data(BlockKey(*child), course_key)
-            if '_subtree_edited_on' not in json_data.setdefault('edit_info', {}):
-                self._compute_subtree_edited_internal(child, child_data, course_key)
-            if child_data['edit_info']['_subtree_edited_on'] > max_date:
-                max_date = child_data['edit_info']['_subtree_edited_on']
-                max_by = child_data['edit_info']['_subtree_edited_by']
-
-        json_data['edit_info']['_subtree_edited_on'] = max_date
-        json_data['edit_info']['_subtree_edited_by'] = max_by
+            if block_data.edit_info._subtree_edited_on is None:
+                self._compute_subtree_edited_internal(child_data, course_key)
+            if child_data.edit_info._subtree_edited_on > max_date:
+                max_date = child_data.edit_info._subtree_edited_on
+                max_date_by = child_data.edit_info._subtree_edited_by
+
+        block_data.edit_info._subtree_edited_on = max_date
+        block_data.edit_info._subtree_edited_by = max_date_by

common/lib/xmodule/xmodule/modulestore/split_mongo/id_manager.py

@@ -26,7 +26,7 @@ class SplitMongoIdManager(OpaqueKeyReader, AsideKeyGenerator):  # pylint: disabl
             block_key = BlockKey.from_usage_key(usage_id)
             module_data = self._cds.get_module_data(block_key, usage_id.course_key)
 
-            if 'definition' in module_data:
-                return DefinitionLocator(usage_id.block_type, module_data['definition'])
+            if module_data.definition is not None:
+                return DefinitionLocator(usage_id.block_type, module_data.definition)
             else:
                 raise ValueError("All non-local blocks should have a definition specified")

common/lib/xmodule/xmodule/modulestore/split_mongo/mongo_connection.py

@@ -10,7 +10,8 @@ from pymongo.errors import DuplicateKeyError  # pylint: disable=unused-import
 
 from contracts import check, new_contract
 from xmodule.exceptions import HeartbeatFailure
-from xmodule.modulestore.split_mongo import BlockKey, BlockData
+from xmodule.modulestore import BlockData
+from xmodule.modulestore.split_mongo import BlockKey
 import datetime
 import pytz
 
@@ -37,7 +38,7 @@ def structure_from_mongo(structure):
     for block in structure['blocks']:
         if 'children' in block['fields']:
             block['fields']['children'] = [BlockKey(*child) for child in block['fields']['children']]
-        new_blocks[BlockKey(block['block_type'], block.pop('block_id'))] = BlockData(block)
+        new_blocks[BlockKey(block['block_type'], block.pop('block_id'))] = BlockData(**block)
     structure['blocks'] = new_blocks
 
     return structure
@@ -54,8 +55,8 @@ def structure_to_mongo(structure):
     check('BlockKey', structure['root'])
     check('dict(BlockKey: BlockData)', structure['blocks'])
     for block in structure['blocks'].itervalues():
-        if 'children' in block['fields']:
-            check('list(BlockKey)', block['fields']['children'])
+        if 'children' in block.fields:
+            check('list(BlockKey)', block.fields['children'])
 
     new_structure = dict(structure)
     new_structure['blocks'] = []

common/lib/xmodule/xmodule/modulestore/split_mongo/split_draft.py

@@ -324,9 +324,9 @@ class DraftVersioningModuleStore(SplitMongoModuleStore, ModuleStoreDraftAndPubli
                 return True
 
             # check the children in the draft
-            if 'children' in draft_block.setdefault('fields', {}):
+            if 'children' in draft_block.fields:
                 return any(
-                    [has_changes_subtree(child_block_id) for child_block_id in draft_block['fields']['children']]
+                    [has_changes_subtree(child_block_id) for child_block_id in draft_block.fields['children']]
                 )
 
             return False
@@ -410,7 +410,7 @@ class DraftVersioningModuleStore(SplitMongoModuleStore, ModuleStoreDraftAndPubli
                     self._get_block_from_structure(published_course_structure, root_block_id)
                 )
                 block = self._get_block_from_structure(new_structure, root_block_id)
-                for child_block_id in block.setdefault('fields', {}).get('children', []):
+                for child_block_id in block.fields.get('children', []):
                     copy_from_published(child_block_id)
 
             copy_from_published(BlockKey.from_usage_key(location))
@@ -472,7 +472,8 @@ class DraftVersioningModuleStore(SplitMongoModuleStore, ModuleStoreDraftAndPubli
         """
         Return the version of the given database representation of a block.
         """
-        return block['edit_info'].get('source_version', block['edit_info']['update_version'])
+        source_version = block.edit_info.source_version
+        return source_version if source_version is not None else block.edit_info.update_version
 
     def import_xblock(self, user_id, course_key, block_type, block_id, fields=None, runtime=None, **kwargs):
         """
@@ -505,8 +506,8 @@ class DraftVersioningModuleStore(SplitMongoModuleStore, ModuleStoreDraftAndPubli
         """
         published_block = self._get_head(xblock, ModuleStoreEnum.BranchName.published)
         if published_block is not None:
-            setattr(xblock, '_published_by', published_block['edit_info']['edited_by'])
-            setattr(xblock, '_published_on', published_block['edit_info']['edited_on'])
+            setattr(xblock, '_published_by', published_block.edit_info.edited_by)
+            setattr(xblock, '_published_on', published_block.edit_info.edited_on)
 
     @contract(asset_key='AssetKey')
     def find_asset_metadata(self, asset_key, **kwargs):

common/lib/xmodule/xmodule/modulestore/tests/test_cross_modulestore_import_export.py

@@ -101,7 +101,7 @@ class MongoModulestoreBuilder(object):
         fs_root = mkdtemp()
 
         # pylint: disable=attribute-defined-outside-init
-        self.modulestore = DraftModuleStore(
+        modulestore = DraftModuleStore(
             contentstore,
             doc_store_config,
             fs_root,
@@ -110,13 +110,13 @@ class MongoModulestoreBuilder(object):
             metadata_inheritance_cache_subsystem=MemoryCache(),
             xblock_mixins=XBLOCK_MIXINS,
         )
-        self.modulestore.ensure_indexes()
+        modulestore.ensure_indexes()
 
         try:
-            yield self.modulestore
+            yield modulestore
         finally:
             # Delete the created database
-            self.modulestore._drop_database()  # pylint: disable=protected-access
+            modulestore._drop_database()  # pylint: disable=protected-access
 
             # Delete the created directory on the filesystem
             rmtree(fs_root, ignore_errors=True)
@@ -124,12 +124,6 @@ class MongoModulestoreBuilder(object):
     def __repr__(self):
         return 'MongoModulestoreBuilder()'
 
-    def asset_collection(self):
-        """
-        Returns the collection storing the asset metadata.
-        """
-        return self.modulestore.asset_collection
-
 
 class VersioningModulestoreBuilder(object):
     """
@@ -213,7 +207,7 @@ class MixedModulestoreBuilder(object):
         """
         self.store_builders = store_builders
         self.mappings = mappings or {}
-        self.modulestore = None
+        self.mixed_modulestore = None
 
     @contextmanager
     def build(self, contentstore):
@@ -235,7 +229,7 @@ class MixedModulestoreBuilder(object):
             # Generate a fake list of stores to give the already generated stores appropriate names
             stores = [{'NAME': name, 'ENGINE': 'This space deliberately left blank'} for name in names]
 
-            self.modulestore = MixedModuleStore(
+            self.mixed_modulestore = MixedModuleStore(
                 contentstore,
                 self.mappings,
                 stores,
@@ -243,7 +237,7 @@ class MixedModulestoreBuilder(object):
                 xblock_mixins=XBLOCK_MIXINS,
             )
 
-            yield self.modulestore
+            yield self.mixed_modulestore
 
     def __repr__(self):
         return 'MixedModulestoreBuilder({!r}, {!r})'.format(self.store_builders, self.mappings)
@@ -252,7 +246,7 @@ class MixedModulestoreBuilder(object):
         """
         Returns the collection storing the asset metadata.
         """
-        all_stores = self.modulestore.modulestores
+        all_stores = self.mixed_modulestore.modulestores
         if len(all_stores) > 1:
             return None
 

common/lib/xmodule/xmodule/modulestore/tests/test_split_modulestore.py

@@ -1580,7 +1580,7 @@ class TestCourseCreation(SplitModuleTest):
         self.assertIsNotNone(db_structure, "Didn't find course")
         self.assertNotIn(BlockKey('course', 'course'), db_structure['blocks'])
         self.assertIn(BlockKey('chapter', 'top'), db_structure['blocks'])
-        self.assertEqual(db_structure['blocks'][BlockKey('chapter', 'top')]['block_type'], 'chapter')
+        self.assertEqual(db_structure['blocks'][BlockKey('chapter', 'top')].block_type, 'chapter')
 
     def test_create_id_dupe(self):
         """

common/lib/xmodule/xmodule/tests/__init__.py

@@ -11,6 +11,7 @@ import json
 import os
 import pprint
 import unittest
+import inspect
 
 from contextlib import contextmanager
 from lazy import lazy
@@ -222,19 +223,12 @@ class BulkAssertionManager(object):
     the failures at once, rather than only seeing single failures.
     """
     def __init__(self, test_case):
-        self._equal_expected = []
-        self._equal_actual = []
+        self._equal_assertions = []
         self._test_case = test_case
 
-    def assertEqual(self, expected, actual, description=None):
-        if description is None:
-            description = u"{!r} does not equal {!r}".format(expected, actual)
-        if expected != actual:
-            self._equal_expected.append((description, expected))
-            self._equal_actual.append((description, actual))
-
     def run_assertions(self):
-        super(BulkAssertionTest, self._test_case).assertEqual(self._equal_expected, self._equal_actual)
+        if len(self._equal_assertions) > 0:
+            raise AssertionError(self._equal_assertions)
 
 
 class BulkAssertionTest(unittest.TestCase):
@@ -262,7 +256,15 @@ class BulkAssertionTest(unittest.TestCase):
 
     def assertEqual(self, expected, actual, message=None):
         if self._manager is not None:
-            self._manager.assertEqual(expected, actual, message)
+            try:
+                super(BulkAssertionTest, self).assertEqual(expected, actual, message)
+            except Exception as error:  # pylint: disable=broad-except
+                exc_stack = inspect.stack()[1]
+                if message is not None:
+                    msg = '{} -> {}:{} -> {}'.format(message, exc_stack[1], exc_stack[2], unicode(error))
+                else:
+                    msg = '{}:{} -> {}'.format(exc_stack[1], exc_stack[2], unicode(error))
+                self._manager._equal_assertions.append(msg)  # pylint: disable=protected-access
         else:
             super(BulkAssertionTest, self).assertEqual(expected, actual, message)
     assertEquals = assertEqual

common/lib/xmodule/xmodule/tests/test_library_content.py

@@ -457,6 +457,7 @@ class TestLibraryContentAnalytics(LibraryContentTest):
         # except for one of the two already assigned to the student:
         keep_block_key = initial_blocks_assigned[0].location
         keep_block_lib_usage_key, keep_block_lib_version = self.store.get_block_original_usage(keep_block_key)
+        self.assertIsNotNone(keep_block_lib_usage_key)
         deleted_block_key = initial_blocks_assigned[1].location
         self.library.children = [keep_block_lib_usage_key]
         self.store.update_item(self.library, self.user_id)