diff --git a/common/lib/xmodule/xmodule/course_module.py b/common/lib/xmodule/xmodule/course_module.py index 832791ba59..5a5066c344 100644 --- a/common/lib/xmodule/xmodule/course_module.py +++ b/common/lib/xmodule/xmodule/course_module.py @@ -919,7 +919,7 @@ class CourseModule(CourseFields, SequenceModule): # pylint: disable=abstract-me """ -class CourseDescriptor(CourseFields, LicenseMixin, SequenceDescriptor): +class CourseDescriptor(CourseFields, SequenceDescriptor, LicenseMixin): """ The descriptor for the course XModule """ diff --git a/common/lib/xmodule/xmodule/error_module.py b/common/lib/xmodule/xmodule/error_module.py index 066a1ad38b..11c0c28720 100644 --- a/common/lib/xmodule/xmodule/error_module.py +++ b/common/lib/xmodule/xmodule/error_module.py @@ -80,7 +80,18 @@ class ErrorDescriptor(ErrorFields, XModuleDescriptor): return u'' @classmethod - def _construct(cls, system, contents, error_msg, location): + def _construct(cls, system, contents, error_msg, location, for_parent=None): + """ + Build a new ErrorDescriptor. using ``system``. + + Arguments: + system (:class:`DescriptorSystem`): The :class:`DescriptorSystem` used + to construct the XBlock that had an error. + contents (unicode): An encoding of the content of the xblock that had an error. + error_msg (unicode): A message describing the error. + location (:class:`UsageKey`): The usage key of the XBlock that had an error. + for_parent (:class:`XBlock`): Optional. The parent of this error block. + """ if error_msg is None: # this string is not marked for translation because we don't have @@ -110,6 +121,7 @@ class ErrorDescriptor(ErrorFields, XModuleDescriptor): # real scope keys ScopeIds(None, 'error', location, location), field_data, + for_parent=for_parent, ) def get_context(self): @@ -139,6 +151,7 @@ class ErrorDescriptor(ErrorFields, XModuleDescriptor): str(descriptor), error_msg, location=descriptor.location, + for_parent=descriptor.get_parent() if descriptor.has_cached_parent else None ) @classmethod diff --git a/common/lib/xmodule/xmodule/modulestore/mongo/base.py b/common/lib/xmodule/xmodule/modulestore/mongo/base.py index 9a4ba7c6f1..d7ef5d542b 100644 --- a/common/lib/xmodule/xmodule/modulestore/mongo/base.py +++ b/common/lib/xmodule/xmodule/modulestore/mongo/base.py @@ -223,7 +223,7 @@ class CachingDescriptorSystem(MakoDescriptorSystem, EditInfoRuntimeMixin): self.course_id = course_key self.cached_metadata = cached_metadata - def load_item(self, location): + def load_item(self, location, for_parent=None): # pylint: disable=method-hidden """ Return an XModule instance for the specified location """ @@ -292,7 +292,7 @@ class CachingDescriptorSystem(MakoDescriptorSystem, EditInfoRuntimeMixin): field_data = KvsFieldData(kvs) scope_ids = ScopeIds(None, category, location, location) - module = self.construct_xblock_from_class(class_, scope_ids, field_data) + module = self.construct_xblock_from_class(class_, scope_ids, field_data, for_parent=for_parent) if self.cached_metadata is not None: # parent container pointers don't differentiate between draft and non-draft # so when we do the lookup, we should do so with a non-draft location @@ -883,7 +883,8 @@ class MongoModuleStore(ModuleStoreDraftAndPublished, ModuleStoreWriteBase, Mongo apply_cached_metadata=bool, using_descriptor_system="None|CachingDescriptorSystem" ) - def _load_item(self, course_key, item, data_cache, apply_cached_metadata=True, using_descriptor_system=None): + def _load_item(self, course_key, item, data_cache, + apply_cached_metadata=True, using_descriptor_system=None, for_parent=None): """ Load an XModuleDescriptor from item, using the children stored in data_cache @@ -898,6 +899,7 @@ class MongoModuleStore(ModuleStoreDraftAndPublished, ModuleStoreWriteBase, Mongo purposes. using_descriptor_system (CachingDescriptorSystem): The existing CachingDescriptorSystem to add data to, and to load the XBlocks from. + for_parent (:class:`XBlock`): The parent of the XBlock being loaded. """ course_key = self.fill_in_run(course_key) location = Location._from_deprecated_son(item['location'], course_key.run) @@ -942,9 +944,9 @@ class MongoModuleStore(ModuleStoreDraftAndPublished, ModuleStoreWriteBase, Mongo system.module_data.update(data_cache) system.cached_metadata.update(cached_metadata) - return system.load_item(location) + return system.load_item(location, for_parent=for_parent) - def _load_items(self, course_key, items, depth=0, using_descriptor_system=None): + def _load_items(self, course_key, items, depth=0, using_descriptor_system=None, for_parent=None): """ Load a list of xmodules from the data in items, with children cached up to specified depth @@ -960,7 +962,8 @@ class MongoModuleStore(ModuleStoreDraftAndPublished, ModuleStoreWriteBase, Mongo item, data_cache, using_descriptor_system=using_descriptor_system, - apply_cached_metadata=self._should_apply_cached_metadata(item, depth) + apply_cached_metadata=self._should_apply_cached_metadata(item, depth), + for_parent=for_parent, ) for item in items ] @@ -1078,7 +1081,7 @@ class MongoModuleStore(ModuleStoreDraftAndPublished, ModuleStoreWriteBase, Mongo except ItemNotFoundError: return False - def get_item(self, usage_key, depth=0, using_descriptor_system=None): + def get_item(self, usage_key, depth=0, using_descriptor_system=None, for_parent=None, **kwargs): """ Returns an XModuleDescriptor instance for the item at location. @@ -1101,7 +1104,8 @@ class MongoModuleStore(ModuleStoreDraftAndPublished, ModuleStoreWriteBase, Mongo usage_key.course_key, [item], depth, - using_descriptor_system=using_descriptor_system + using_descriptor_system=using_descriptor_system, + for_parent=for_parent, )[0] return module @@ -1293,6 +1297,7 @@ class MongoModuleStore(ModuleStoreDraftAndPublished, ModuleStoreWriteBase, Mongo # so we use the location for both. ScopeIds(None, block_type, location, location), dbmodel, + for_parent=kwargs.get('for_parent'), ) if fields is not None: for key, value in fields.iteritems(): @@ -1341,11 +1346,16 @@ class MongoModuleStore(ModuleStoreDraftAndPublished, ModuleStoreWriteBase, Mongo block_id: a unique identifier for the new item. If not supplied, a new identifier will be generated """ - xblock = self.create_item(user_id, parent_usage_key.course_key, block_type, block_id=block_id, **kwargs) # attach to parent if given - if 'detached' not in xblock._class_tags: - parent = self.get_item(parent_usage_key) + parent = None + if parent_usage_key is not None: + parent = self.get_item(parent_usage_key) + kwargs.setdefault('for_parent', parent) + + xblock = self.create_item(user_id, parent_usage_key.course_key, block_type, block_id=block_id, **kwargs) + + if parent is not None and 'detached' not in xblock._class_tags: # Originally added to support entrance exams (settings.FEATURES.get('ENTRANCE_EXAMS')) if kwargs.get('position') is None: parent.children.append(xblock.location) diff --git a/common/lib/xmodule/xmodule/modulestore/mongo/draft.py b/common/lib/xmodule/xmodule/modulestore/mongo/draft.py index 814058d145..8c181b2639 100644 --- a/common/lib/xmodule/xmodule/modulestore/mongo/draft.py +++ b/common/lib/xmodule/xmodule/modulestore/mongo/draft.py @@ -82,12 +82,14 @@ class DraftModuleStore(MongoModuleStore): """ def get_published(): return wrap_draft(super(DraftModuleStore, self).get_item( - usage_key, depth=depth, using_descriptor_system=using_descriptor_system + usage_key, depth=depth, using_descriptor_system=using_descriptor_system, + for_parent=kwargs.get('for_parent'), )) def get_draft(): return wrap_draft(super(DraftModuleStore, self).get_item( - as_draft(usage_key), depth=depth, using_descriptor_system=using_descriptor_system + as_draft(usage_key), depth=depth, using_descriptor_system=using_descriptor_system, + for_parent=kwargs.get('for_parent') )) # return the published version if ModuleStoreEnum.RevisionOption.published_only is requested 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 dff8b0d853..f8f1dbe54e 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 @@ -227,6 +227,7 @@ class CachingDescriptorSystem(MakoDescriptorSystem, EditInfoRuntimeMixin): class_, ScopeIds(None, block_key.type, definition_id, block_locator), field_data, + for_parent=kwargs.get('for_parent') ) except Exception: # pylint: disable=broad-except log.warning("Failed to load descriptor", exc_info=True) diff --git a/common/lib/xmodule/xmodule/modulestore/tests/factories.py b/common/lib/xmodule/xmodule/modulestore/tests/factories.py index cea8c2ebc4..abfc1f9664 100644 --- a/common/lib/xmodule/xmodule/modulestore/tests/factories.py +++ b/common/lib/xmodule/xmodule/modulestore/tests/factories.py @@ -2,12 +2,15 @@ Factories for use in tests of XBlocks. """ +import functools import inspect import pprint -import threading -from uuid import uuid4 -from decorator import contextmanager import pymongo.message +import threading +import traceback +from collections import defaultdict +from decorator import contextmanager +from uuid import uuid4 from factory import Factory, Sequence, lazy_attribute_sequence, lazy_attribute from factory.containers import CyclicDefinitionError @@ -320,47 +323,160 @@ def check_number_of_calls(object_with_method, method_name, maximum_calls, minimu return check_sum_of_calls(object_with_method, [method_name], maximum_calls, minimum_calls) +class StackTraceCounter(object): + """ + A class that counts unique stack traces underneath a particular stack frame. + """ + def __init__(self, stack_depth, include_arguments=True): + """ + Arguments: + stack_depth (int): The number of stack frames above this constructor to capture. + include_arguments (bool): Whether to store the arguments that are passed + when capturing a stack trace. + """ + self.include_arguments = include_arguments + self._top_of_stack = traceback.extract_stack(limit=stack_depth)[0] + + if self.include_arguments: + self._stacks = defaultdict(lambda: defaultdict(int)) + else: + self._stacks = defaultdict(int) + + def capture_stack(self, args, kwargs): + """ + Record the stack frames starting at the caller of this method, and + ending at the top of the stack as defined by the ``stack_depth``. + + Arguments: + args: The positional arguments to capture at this stack frame + kwargs: The keyword arguments to capture at this stack frame + """ + # pylint: disable=broad-except + + stack = traceback.extract_stack()[:-2] + + if self._top_of_stack in stack: + stack = stack[stack.index(self._top_of_stack):] + + if self.include_arguments: + safe_args = [] + for arg in args: + try: + safe_args.append(repr(arg)) + except Exception as exc: + safe_args.append('