Commit 68325f1c by John Eskew Committed by Calen Pennington

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
parent d9dc82ba
...@@ -21,3 +21,78 @@ class BlockKey(namedtuple('BlockKey', 'type id')): ...@@ -21,3 +21,78 @@ class BlockKey(namedtuple('BlockKey', 'type id')):
CourseEnvelope = namedtuple('CourseEnvelope', 'course_key structure') 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): ...@@ -117,10 +117,10 @@ class CachingDescriptorSystem(MakoDescriptorSystem, EditInfoRuntimeMixin):
if cached_module: if cached_module:
return 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')) class_ = self.load_block_type(block_data.get('block_type'))
block = self.xblock_from_json(class_, course_key, block_key, json_data, course_entry_override, **kwargs) 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) self.modulestore.cache_block(course_key, version_guid, block_key, block)
return block return block
...@@ -154,26 +154,27 @@ class CachingDescriptorSystem(MakoDescriptorSystem, EditInfoRuntimeMixin): ...@@ -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 # 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. # 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") @contract(block_key="BlockKey | None")
def xblock_from_json( def xblock_from_json(self, class_, course_key, block_key, block_data, course_entry_override=None, **kwargs):
self, class_, course_key, block_key, json_data, course_entry_override=None, **kwargs """
): Load and return block info.
"""
if course_entry_override is None: if course_entry_override is None:
course_entry_override = self.course_entry course_entry_override = self.course_entry
else: else:
# most recent retrieval is most likely the right one for next caller (see comment above fn) # 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) 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 no usage id is provided, generate an in-memory id
if block_key is None: 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( convert_fields = lambda field: self.modulestore.convert_references_to_keys(
course_key, class_, field, self.course_entry.structure['blocks'], 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( definition_loader = DefinitionLazyLoader(
self.modulestore, self.modulestore,
course_key, course_key,
...@@ -194,8 +195,8 @@ class CachingDescriptorSystem(MakoDescriptorSystem, EditInfoRuntimeMixin): ...@@ -194,8 +195,8 @@ class CachingDescriptorSystem(MakoDescriptorSystem, EditInfoRuntimeMixin):
block_id=block_key.id, block_id=block_key.id,
) )
converted_fields = convert_fields(json_data.get('fields', {})) converted_fields = convert_fields(block_data.get('fields', {}))
converted_defaults = convert_fields(json_data.get('defaults', {})) converted_defaults = convert_fields(block_data.get('defaults', {}))
if block_key in self._parent_map: if block_key in self._parent_map:
parent_key = self._parent_map[block_key] parent_key = self._parent_map[block_key]
parent = course_key.make_usage_key(parent_key.type, parent_key.id) parent = course_key.make_usage_key(parent_key.type, parent_key.id)
...@@ -223,7 +224,7 @@ class CachingDescriptorSystem(MakoDescriptorSystem, EditInfoRuntimeMixin): ...@@ -223,7 +224,7 @@ class CachingDescriptorSystem(MakoDescriptorSystem, EditInfoRuntimeMixin):
except Exception: except Exception:
log.warning("Failed to load descriptor", exc_info=True) log.warning("Failed to load descriptor", exc_info=True)
return ErrorDescriptor.from_json( return ErrorDescriptor.from_json(
json_data, block_data,
self, self,
course_entry_override.course_key.make_usage_key( course_entry_override.course_key.make_usage_key(
block_type='error', block_type='error',
...@@ -232,9 +233,9 @@ class CachingDescriptorSystem(MakoDescriptorSystem, EditInfoRuntimeMixin): ...@@ -232,9 +233,9 @@ class CachingDescriptorSystem(MakoDescriptorSystem, EditInfoRuntimeMixin):
error_msg=exc_info_to_str(sys.exc_info()) error_msg=exc_info_to_str(sys.exc_info())
) )
edit_info = json_data.get('edit_info', {}) edit_info = block_data.get('edit_info', {})
module._edited_by = edit_info.get('edited_by') module._edited_by = edit_info.get('edited_by') # pylint: disable=protected-access
module._edited_on = edit_info.get('edited_on') module._edited_on = edit_info.get('edited_on') # pylint: disable=protected-access
module.previous_version = edit_info.get('previous_version') module.previous_version = edit_info.get('previous_version')
module.update_version = edit_info.get('update_version') module.update_version = edit_info.get('update_version')
module.source_version = edit_info.get('source_version', None) module.source_version = edit_info.get('source_version', None)
......
...@@ -8,13 +8,16 @@ import pymongo ...@@ -8,13 +8,16 @@ import pymongo
# Import this just to export it # Import this just to export it
from pymongo.errors import DuplicateKeyError # pylint: disable=unused-import 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.exceptions import HeartbeatFailure
from xmodule.modulestore.split_mongo import BlockKey from xmodule.modulestore.split_mongo import BlockKey, BlockData
import datetime import datetime
import pytz import pytz
new_contract('BlockData', BlockData)
def structure_from_mongo(structure): def structure_from_mongo(structure):
""" """
Converts the 'blocks' key from a list [block_data] to a map Converts the 'blocks' key from a list [block_data] to a map
...@@ -34,7 +37,7 @@ def structure_from_mongo(structure): ...@@ -34,7 +37,7 @@ def structure_from_mongo(structure):
for block in structure['blocks']: for block in structure['blocks']:
if 'children' in block['fields']: if 'children' in block['fields']:
block['fields']['children'] = [BlockKey(*child) for child in block['fields']['children']] 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 structure['blocks'] = new_blocks
return structure return structure
...@@ -49,7 +52,7 @@ def structure_to_mongo(structure): ...@@ -49,7 +52,7 @@ def structure_to_mongo(structure):
directly into mongo. directly into mongo.
""" """
check('BlockKey', structure['root']) check('BlockKey', structure['root'])
check('dict(BlockKey: dict)', structure['blocks']) check('dict(BlockKey: BlockData)', structure['blocks'])
for block in structure['blocks'].itervalues(): for block in structure['blocks'].itervalues():
if 'children' in block['fields']: if 'children' in block['fields']:
check('list(BlockKey)', block['fields']['children']) check('list(BlockKey)', block['fields']['children'])
...@@ -58,7 +61,7 @@ def structure_to_mongo(structure): ...@@ -58,7 +61,7 @@ def structure_to_mongo(structure):
new_structure['blocks'] = [] new_structure['blocks'] = []
for block_key, block in structure['blocks'].iteritems(): 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.setdefault('block_type', block_key.type)
new_block['block_id'] = block_key.id new_block['block_id'] = block_key.id
new_structure['blocks'].append(new_block) new_structure['blocks'].append(new_block)
......
...@@ -78,7 +78,7 @@ from xmodule.modulestore import ( ...@@ -78,7 +78,7 @@ from xmodule.modulestore import (
from ..exceptions import ItemNotFoundError from ..exceptions import ItemNotFoundError
from .caching_descriptor_system import CachingDescriptorSystem from .caching_descriptor_system import CachingDescriptorSystem
from xmodule.modulestore.split_mongo.mongo_connection import MongoConnection, DuplicateKeyError 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 xmodule.error_module import ErrorDescriptor
from collections import defaultdict from collections import defaultdict
from types import NoneType from types import NoneType
...@@ -676,8 +676,7 @@ class SplitMongoModuleStore(SplitBulkWriteMixin, ModuleStoreWriteBase): ...@@ -676,8 +676,7 @@ class SplitMongoModuleStore(SplitBulkWriteMixin, ModuleStoreWriteBase):
new_module_data = {} new_module_data = {}
for block_id in base_block_ids: for block_id in base_block_ids:
new_module_data = self.descendants( new_module_data = self.descendants(
# copy or our changes like setting 'definition_loaded' will affect the active bulk operation data system.course_entry.structure['blocks'],
copy.deepcopy(system.course_entry.structure['blocks']),
block_id, block_id,
depth, depth,
new_module_data new_module_data
...@@ -2332,7 +2331,7 @@ class SplitMongoModuleStore(SplitBulkWriteMixin, ModuleStoreWriteBase): ...@@ -2332,7 +2331,7 @@ class SplitMongoModuleStore(SplitBulkWriteMixin, ModuleStoreWriteBase):
return result 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): def _remove_subtree(self, block_key, blocks):
""" """
Remove the subtree rooted at block_key Remove the subtree rooted at block_key
...@@ -2912,6 +2911,7 @@ class SplitMongoModuleStore(SplitBulkWriteMixin, ModuleStoreWriteBase): ...@@ -2912,6 +2911,7 @@ class SplitMongoModuleStore(SplitBulkWriteMixin, ModuleStoreWriteBase):
self._delete_if_true_orphan(BlockKey(*child), structure) self._delete_if_true_orphan(BlockKey(*child), structure)
del structure['blocks'][orphan] 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): 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. Create the core document structure for a block.
...@@ -2936,20 +2936,20 @@ class SplitMongoModuleStore(SplitBulkWriteMixin, ModuleStoreWriteBase): ...@@ -2936,20 +2936,20 @@ class SplitMongoModuleStore(SplitBulkWriteMixin, ModuleStoreWriteBase):
} }
if block_defaults: if block_defaults:
document['defaults'] = 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): 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. be a json dict key.
""" """
return structure['blocks'].get(block_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): 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. be a json dict key.
""" """
structure['blocks'][block_key] = content structure['blocks'][block_key] = content
......
Markdown is supported
0% or
You are about to add 0 people to the discussion. Proceed with caution.
Finish editing this message first!
Please register or to comment