Add BlockData class to wrap all block data loaded from the Split
modulestore, in order to separate serializable block data from out-of-band, non-saved, temporary metadata. Eliminate the deepcopy causing performance problems with import/export. https://openedx.atlassian.net/browse/PLAT-416
This commit is contained in:
@@ -21,3 +21,78 @@ 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
|
||||
|
||||
@@ -117,10 +117,10 @@ class CachingDescriptorSystem(MakoDescriptorSystem, EditInfoRuntimeMixin):
|
||||
if cached_module:
|
||||
return cached_module
|
||||
|
||||
json_data = self.get_module_data(block_key, course_key)
|
||||
block_data = self.get_module_data(block_key, course_key)
|
||||
|
||||
class_ = self.load_block_type(json_data.get('block_type'))
|
||||
block = self.xblock_from_json(class_, course_key, block_key, json_data, course_entry_override, **kwargs)
|
||||
class_ = self.load_block_type(block_data.get('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
|
||||
|
||||
@@ -154,26 +154,27 @@ class CachingDescriptorSystem(MakoDescriptorSystem, EditInfoRuntimeMixin):
|
||||
# pointing to the same structure, the access is likely to be chunky enough that the last known container
|
||||
# is the intended one when not given a course_entry_override; thus, the caching of the last branch/course id.
|
||||
@contract(block_key="BlockKey | None")
|
||||
def xblock_from_json(
|
||||
self, class_, course_key, block_key, json_data, course_entry_override=None, **kwargs
|
||||
):
|
||||
def xblock_from_json(self, class_, course_key, block_key, block_data, course_entry_override=None, **kwargs):
|
||||
"""
|
||||
Load and return block info.
|
||||
"""
|
||||
if course_entry_override is None:
|
||||
course_entry_override = self.course_entry
|
||||
else:
|
||||
# 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 = json_data.get('definition')
|
||||
definition_id = block_data.get('definition')
|
||||
|
||||
# If no usage id is provided, generate an in-memory id
|
||||
if block_key is None:
|
||||
block_key = BlockKey(json_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 json_data.get('definition_loaded', False):
|
||||
if definition_id is not None and not block_data['definition_loaded']:
|
||||
definition_loader = DefinitionLazyLoader(
|
||||
self.modulestore,
|
||||
course_key,
|
||||
@@ -194,8 +195,8 @@ class CachingDescriptorSystem(MakoDescriptorSystem, EditInfoRuntimeMixin):
|
||||
block_id=block_key.id,
|
||||
)
|
||||
|
||||
converted_fields = convert_fields(json_data.get('fields', {}))
|
||||
converted_defaults = convert_fields(json_data.get('defaults', {}))
|
||||
converted_fields = convert_fields(block_data.get('fields', {}))
|
||||
converted_defaults = convert_fields(block_data.get('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)
|
||||
@@ -223,7 +224,7 @@ class CachingDescriptorSystem(MakoDescriptorSystem, EditInfoRuntimeMixin):
|
||||
except Exception:
|
||||
log.warning("Failed to load descriptor", exc_info=True)
|
||||
return ErrorDescriptor.from_json(
|
||||
json_data,
|
||||
block_data,
|
||||
self,
|
||||
course_entry_override.course_key.make_usage_key(
|
||||
block_type='error',
|
||||
@@ -232,9 +233,9 @@ class CachingDescriptorSystem(MakoDescriptorSystem, EditInfoRuntimeMixin):
|
||||
error_msg=exc_info_to_str(sys.exc_info())
|
||||
)
|
||||
|
||||
edit_info = json_data.get('edit_info', {})
|
||||
module._edited_by = edit_info.get('edited_by')
|
||||
module._edited_on = edit_info.get('edited_on')
|
||||
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)
|
||||
|
||||
@@ -8,13 +8,16 @@ import pymongo
|
||||
# Import this just to export it
|
||||
from pymongo.errors import DuplicateKeyError # pylint: disable=unused-import
|
||||
|
||||
from contracts import check
|
||||
from contracts import check, new_contract
|
||||
from xmodule.exceptions import HeartbeatFailure
|
||||
from xmodule.modulestore.split_mongo import BlockKey
|
||||
from xmodule.modulestore.split_mongo import BlockKey, BlockData
|
||||
import datetime
|
||||
import pytz
|
||||
|
||||
|
||||
new_contract('BlockData', BlockData)
|
||||
|
||||
|
||||
def structure_from_mongo(structure):
|
||||
"""
|
||||
Converts the 'blocks' key from a list [block_data] to a map
|
||||
@@ -34,7 +37,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'))] = block
|
||||
new_blocks[BlockKey(block['block_type'], block.pop('block_id'))] = BlockData(block)
|
||||
structure['blocks'] = new_blocks
|
||||
|
||||
return structure
|
||||
@@ -49,7 +52,7 @@ def structure_to_mongo(structure):
|
||||
directly into mongo.
|
||||
"""
|
||||
check('BlockKey', structure['root'])
|
||||
check('dict(BlockKey: dict)', structure['blocks'])
|
||||
check('dict(BlockKey: BlockData)', structure['blocks'])
|
||||
for block in structure['blocks'].itervalues():
|
||||
if 'children' in block['fields']:
|
||||
check('list(BlockKey)', block['fields']['children'])
|
||||
@@ -58,7 +61,7 @@ def structure_to_mongo(structure):
|
||||
new_structure['blocks'] = []
|
||||
|
||||
for block_key, block in structure['blocks'].iteritems():
|
||||
new_block = dict(block)
|
||||
new_block = dict(block.to_storable())
|
||||
new_block.setdefault('block_type', block_key.type)
|
||||
new_block['block_id'] = block_key.id
|
||||
new_structure['blocks'].append(new_block)
|
||||
|
||||
@@ -78,7 +78,7 @@ from xmodule.modulestore import (
|
||||
from ..exceptions import ItemNotFoundError
|
||||
from .caching_descriptor_system import CachingDescriptorSystem
|
||||
from xmodule.modulestore.split_mongo.mongo_connection import MongoConnection, DuplicateKeyError
|
||||
from xmodule.modulestore.split_mongo import BlockKey, CourseEnvelope
|
||||
from xmodule.modulestore.split_mongo import BlockKey, CourseEnvelope, BlockData
|
||||
from xmodule.error_module import ErrorDescriptor
|
||||
from collections import defaultdict
|
||||
from types import NoneType
|
||||
@@ -676,8 +676,7 @@ class SplitMongoModuleStore(SplitBulkWriteMixin, ModuleStoreWriteBase):
|
||||
new_module_data = {}
|
||||
for block_id in base_block_ids:
|
||||
new_module_data = self.descendants(
|
||||
# copy or our changes like setting 'definition_loaded' will affect the active bulk operation data
|
||||
copy.deepcopy(system.course_entry.structure['blocks']),
|
||||
system.course_entry.structure['blocks'],
|
||||
block_id,
|
||||
depth,
|
||||
new_module_data
|
||||
@@ -2332,7 +2331,7 @@ class SplitMongoModuleStore(SplitBulkWriteMixin, ModuleStoreWriteBase):
|
||||
|
||||
return result
|
||||
|
||||
@contract(block_key=BlockKey, blocks='dict(BlockKey: dict)')
|
||||
@contract(block_key=BlockKey, blocks='dict(BlockKey: BlockData)')
|
||||
def _remove_subtree(self, block_key, blocks):
|
||||
"""
|
||||
Remove the subtree rooted at block_key
|
||||
@@ -2912,6 +2911,7 @@ class SplitMongoModuleStore(SplitBulkWriteMixin, ModuleStoreWriteBase):
|
||||
self._delete_if_true_orphan(BlockKey(*child), structure)
|
||||
del structure['blocks'][orphan]
|
||||
|
||||
@contract(returns=BlockData)
|
||||
def _new_block(self, user_id, category, block_fields, definition_id, new_id, raw=False, block_defaults=None):
|
||||
"""
|
||||
Create the core document structure for a block.
|
||||
@@ -2936,20 +2936,20 @@ class SplitMongoModuleStore(SplitBulkWriteMixin, ModuleStoreWriteBase):
|
||||
}
|
||||
if block_defaults:
|
||||
document['defaults'] = block_defaults
|
||||
return document
|
||||
return BlockData(document)
|
||||
|
||||
@contract(block_key=BlockKey)
|
||||
@contract(block_key=BlockKey, returns='BlockData | None')
|
||||
def _get_block_from_structure(self, structure, block_key):
|
||||
"""
|
||||
Encodes the block id before retrieving it from the structure to ensure it can
|
||||
Encodes the block key before retrieving it from the structure to ensure it can
|
||||
be a json dict key.
|
||||
"""
|
||||
return structure['blocks'].get(block_key)
|
||||
|
||||
@contract(block_key=BlockKey)
|
||||
@contract(block_key=BlockKey, content=BlockData)
|
||||
def _update_block_in_structure(self, structure, block_key, content):
|
||||
"""
|
||||
Encodes the block id before accessing it in the structure to ensure it can
|
||||
Encodes the block key before accessing it in the structure to ensure it can
|
||||
be a json dict key.
|
||||
"""
|
||||
structure['blocks'][block_key] = content
|
||||
|
||||
Reference in New Issue
Block a user