From aa07355e8758f6bc06b9619874fc13e5d226c4c5 Mon Sep 17 00:00:00 2001 From: Don Mitchell Date: Thu, 30 Oct 2014 14:22:24 -0400 Subject: [PATCH] Abstract asset methods into own interface class then impl the read ones as NotImplementedError placeholders on xml modulestore PLAT-70 --- .../xmodule/xmodule/modulestore/__init__.py | 1299 +++++++++-------- .../modulestore/tests/test_assetstore.py | 20 +- .../test_cross_modulestore_import_export.py | 28 +- common/lib/xmodule/xmodule/modulestore/xml.py | 49 + 4 files changed, 748 insertions(+), 648 deletions(-) diff --git a/common/lib/xmodule/xmodule/modulestore/__init__.py b/common/lib/xmodule/xmodule/modulestore/__init__.py index 7ae26268e8..a436bd34c0 100644 --- a/common/lib/xmodule/xmodule/modulestore/__init__.py +++ b/common/lib/xmodule/xmodule/modulestore/__init__.py @@ -266,620 +266,10 @@ class BulkOperationsMixin(object): return self._get_bulk_ops_record(course_key, ignore_case).active -class ModuleStoreRead(object): +class ModuleStoreAssetInterface(object): """ - An abstract interface for a database backend that stores XModuleDescriptor - instances and extends read-only functionality + The methods for accessing assets and their metadata """ - - __metaclass__ = ABCMeta - - @abstractmethod - def has_item(self, usage_key): - """ - Returns True if usage_key exists in this ModuleStore. - """ - pass - - @abstractmethod - def get_item(self, usage_key, depth=0, **kwargs): - """ - Returns an XModuleDescriptor instance for the item at location. - - If any segment of the location is None except revision, raises - xmodule.modulestore.exceptions.InsufficientSpecificationError - - If no object is found at that location, raises - xmodule.modulestore.exceptions.ItemNotFoundError - - usage_key: A :class:`.UsageKey` subclass instance - - depth (int): An argument that some module stores may use to prefetch - descendents of the queried modules for more efficient results later - in the request. The depth is counted in the number of calls to - get_children() to cache. None indicates to cache all descendents - """ - pass - - @abstractmethod - def get_course_errors(self, course_key): - """ - Return a list of (msg, exception-or-None) errors that the modulestore - encountered when loading the course at course_id. - - Raises the same exceptions as get_item if the location isn't found or - isn't fully specified. - - Args: - course_key (:class:`.CourseKey`): The course to check for errors - """ - pass - - @abstractmethod - def get_items(self, location, course_id=None, depth=0, qualifiers=None, **kwargs): - """ - Returns a list of XModuleDescriptor instances for the items - that match location. Any element of location that is None is treated - as a wildcard that matches any value - - location: Something that can be passed to Location - - depth: An argument that some module stores may use to prefetch - descendents of the queried modules for more efficient results later - in the request. The depth is counted in the number of calls to - get_children() to cache. None indicates to cache all descendents - """ - pass - - def _block_matches(self, fields_or_xblock, 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) - - 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 - else: - fields = fields_or_xblock - is_xblock = False - - def _is_set_on(key): - """ - Is this key set in fields? (return tuple of boolean and value). A helper which can - handle fields either being the json doc or xblock fields. Is inner function to restrict - use and to access local vars. - """ - 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) - else: - return True, field - - for key, criteria in qualifiers.iteritems(): - is_set, value = _is_set_on(key) - if not is_set: - return False - if not self._value_matches(value, criteria): - return False - 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 - If the criteria is a regex, does the target match it? - 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): - return criteria.search(target) is not None - elif callable(criteria): - return criteria(target) - elif isinstance(criteria, dict) and '$in' in criteria: - # note isn't handling any other things in the dict other than in - return any(self._value_matches(target, test_val) for test_val in criteria['$in']) - elif isinstance(criteria, dict) and '$nin' in criteria: - # note isn't handling any other things in the dict other than nin - return not any(self._value_matches(target, test_val) for test_val in criteria['$nin']) - else: - return criteria == target - - @abstractmethod - def make_course_key(self, org, course, run): - """ - Return a valid :class:`~opaque_keys.edx.keys.CourseKey` for this modulestore - that matches the supplied `org`, `course`, and `run`. - - This key may represent a course that doesn't exist in this modulestore. - """ - pass - - @abstractmethod - def get_courses(self, **kwargs): - ''' - Returns a list containing the top level XModuleDescriptors of the courses - in this modulestore. - ''' - pass - - @abstractmethod - def get_course(self, course_id, depth=0, **kwargs): - ''' - Look for a specific course by its id (:class:`CourseKey`). - Returns the course descriptor, or None if not found. - ''' - pass - - @abstractmethod - def has_course(self, course_id, ignore_case=False, **kwargs): - ''' - Look for a specific course id. Returns whether it exists. - Args: - course_id (CourseKey): - ignore_case (boolean): some modulestores are case-insensitive. Use this flag - to search for whether a potentially conflicting course exists in that case. - ''' - pass - - @abstractmethod - def get_parent_location(self, location, **kwargs): - ''' - Find the location that is the parent of this location in this - course. Needed for path_to_location(). - ''' - pass - - @abstractmethod - def get_orphans(self, course_key, **kwargs): - """ - Get all of the xblocks in the given course which have no parents and are not of types which are - usually orphaned. NOTE: may include xblocks which still have references via xblocks which don't - use children to point to their dependents. - """ - pass - - @abstractmethod - def get_errored_courses(self): - """ - Return a dictionary of course_dir -> [(msg, exception_str)], for each - course_dir where course loading failed. - """ - pass - - @abstractmethod - def get_modulestore_type(self, course_id): - """ - Returns a type which identifies which modulestore is servicing the given - course_id. The return can be either "xml" (for XML based courses) or "mongo" for MongoDB backed courses - """ - pass - - @abstractmethod - def get_courses_for_wiki(self, wiki_slug, **kwargs): - """ - Return the list of courses which use this wiki_slug - :param wiki_slug: the course wiki root slug - :return: list of course keys - """ - pass - - @abstractmethod - def has_published_version(self, xblock): - """ - Returns true if this xblock exists in the published course regardless of whether it's up to date - """ - pass - - @abstractmethod - def close_connections(self): - """ - Closes any open connections to the underlying databases - """ - pass - - @contextmanager - def bulk_operations(self, course_id): - """ - A context manager for notifying the store of bulk operations. This affects only the current thread. - """ - yield - - def ensure_indexes(self): - """ - Ensure that all appropriate indexes are created that are needed by this modulestore, or raise - an exception if unable to. - - This method is intended for use by tests and administrative commands, and not - to be run during server startup. - """ - pass - - -class ModuleStoreWrite(ModuleStoreRead): - """ - An abstract interface for a database backend that stores XModuleDescriptor - instances and extends both read and write functionality - """ - - __metaclass__ = ABCMeta - - @abstractmethod - def update_item(self, xblock, user_id, allow_not_found=False, force=False, **kwargs): - """ - Update the given xblock's persisted repr. Pass the user's unique id which the persistent store - should save with the update if it has that ability. - - :param allow_not_found: whether this method should raise an exception if the given xblock - has not been persisted before. - :param force: fork the structure and don't update the course draftVersion if there's a version - conflict (only applicable to version tracking and conflict detecting persistence stores) - - :raises VersionConflictError: if org, course, run, and version_guid given and the current - version head != version_guid and force is not True. (only applicable to version tracking stores) - """ - pass - - @abstractmethod - def delete_item(self, location, user_id, **kwargs): - """ - Delete an item and its subtree from persistence. Remove the item from any parents (Note, does not - affect parents from other branches or logical branches; thus, in old mongo, deleting something - whose parent cannot be draft, deletes it from both but deleting a component under a draft vertical - only deletes it from the draft. - - Pass the user's unique id which the persistent store - should save with the update if it has that ability. - - :param force: fork the structure and don't update the course draftVersion if there's a version - conflict (only applicable to version tracking and conflict detecting persistence stores) - - :raises VersionConflictError: if org, course, run, and version_guid given and the current - version head != version_guid and force is not True. (only applicable to version tracking stores) - """ - pass - - @abstractmethod - def create_course(self, org, course, run, user_id, fields=None, **kwargs): - """ - Creates and returns the course. - - Args: - org (str): the organization that owns the course - course (str): the name of the course - run (str): the name of the run - user_id: id of the user creating the course - fields (dict): Fields to set on the course at initialization - kwargs: Any optional arguments understood by a subset of modulestores to customize instantiation - - Returns: a CourseDescriptor - """ - pass - - @abstractmethod - def create_item(self, user_id, course_key, block_type, block_id=None, fields=None, **kwargs): - """ - Creates and saves a new item in a course. - - Returns the newly created item. - - Args: - user_id: ID of the user creating and saving the xmodule - course_key: A :class:`~opaque_keys.edx.CourseKey` identifying which course to create - this item in - block_type: The type of block to create - block_id: a unique identifier for the new item. If not supplied, - a new identifier will be generated - fields (dict): A dictionary specifying initial values for some or all fields - in the newly created block - """ - pass - - @abstractmethod - def clone_course(self, source_course_id, dest_course_id, user_id, fields=None): - """ - Sets up source_course_id to point a course with the same content as the desct_course_id. This - operation may be cheap or expensive. It may have to copy all assets and all xblock content or - merely setup new pointers. - - Backward compatibility: this method used to require in some modulestores that dest_course_id - pointed to an empty but already created course. Implementers should support this or should - enable creating the course from scratch. - - Raises: - ItemNotFoundError: if the source course doesn't exist (or any of its xblocks aren't found) - DuplicateItemError: if the destination course already exists (with content in some cases) - """ - pass - - @abstractmethod - def delete_course(self, course_key, user_id, **kwargs): - """ - Deletes the course. It may be a soft or hard delete. It may or may not remove the xblock definitions - depending on the persistence layer and how tightly bound the xblocks are to the course. - - Args: - course_key (CourseKey): which course to delete - user_id: id of the user deleting the course - """ - pass - - @abstractmethod - def _drop_database(self): - """ - A destructive operation to drop the underlying database and close all connections. - Intended to be used by test code for cleanup. - """ - pass - - -class ModuleStoreReadBase(BulkOperationsMixin, ModuleStoreRead): - ''' - Implement interface functionality that can be shared. - ''' - # pylint: disable=W0613 - - def __init__( - self, - contentstore=None, - doc_store_config=None, # ignore if passed up - metadata_inheritance_cache_subsystem=None, request_cache=None, - xblock_mixins=(), xblock_select=None, - # temporary parms to enable backward compatibility. remove once all envs migrated - db=None, collection=None, host=None, port=None, tz_aware=True, user=None, password=None, - # allow lower level init args to pass harmlessly - ** kwargs - ): - ''' - Set up the error-tracking logic. - ''' - super(ModuleStoreReadBase, self).__init__(**kwargs) - self._course_errors = defaultdict(make_error_tracker) # location -> ErrorLog - # TODO move the inheritance_cache_subsystem to classes which use it - self.metadata_inheritance_cache_subsystem = metadata_inheritance_cache_subsystem - self.request_cache = request_cache - self.xblock_mixins = xblock_mixins - self.xblock_select = xblock_select - self.contentstore = contentstore - - def get_course_errors(self, course_key): - """ - Return list of errors for this :class:`.CourseKey`, if any. Raise the same - errors as get_item if course_key isn't present. - """ - # check that item is present and raise the promised exceptions if needed - # TODO (vshnayder): post-launch, make errors properties of items - # self.get_item(location) - assert(isinstance(course_key, CourseKey)) - return self._course_errors[course_key].errors - - def get_errored_courses(self): - """ - Returns an empty dict. - - It is up to subclasses to extend this method if the concept - of errored courses makes sense for their implementation. - """ - return {} - - def get_course(self, course_id, depth=0, **kwargs): - """ - See ModuleStoreRead.get_course - - Default impl--linear search through course list - """ - assert(isinstance(course_id, CourseKey)) - for course in self.get_courses(**kwargs): - if course.id == course_id: - return course - return None - - def has_course(self, course_id, ignore_case=False, **kwargs): - """ - Returns the course_id of the course if it was found, else None - Args: - course_id (CourseKey): - ignore_case (boolean): some modulestores are case-insensitive. Use this flag - to search for whether a potentially conflicting course exists in that case. - """ - # linear search through list - assert(isinstance(course_id, CourseKey)) - if ignore_case: - return next( - ( - c.id for c in self.get_courses() - if c.id.org.lower() == course_id.org.lower() and - c.id.course.lower() == course_id.course.lower() and - c.id.run.lower() == course_id.run.lower() - ), - None - ) - else: - return next( - (c.id for c in self.get_courses() if c.id == course_id), - None - ) - - def has_published_version(self, xblock): - """ - Returns True since this is a read-only store. - """ - return True - - def heartbeat(self): - """ - Is this modulestore ready? - """ - # default is to say yes by not raising an exception - return {'default_impl': True} - - def close_connections(self): - """ - Closes any open connections to the underlying databases - """ - if self.contentstore: - self.contentstore.close_connections() - super(ModuleStoreReadBase, self).close_connections() - - @contextmanager - def default_store(self, store_type): - """ - A context manager for temporarily changing the default store - """ - if self.get_modulestore_type(None) != store_type: - raise ValueError(u"Cannot set default store to type {}".format(store_type)) - yield - - @staticmethod - def memoize_request_cache(func): - """ - Memoize a function call results on the request_cache if there's one. Creates the cache key by - joining the unicode of all the args with &; so, if your arg may use the default &, it may - have false hits - """ - @functools.wraps(func) - def wrapper(self, *args, **kwargs): - """ - Wraps a method to memoize results. - """ - if self.request_cache: - cache_key = '&'.join([hashvalue(arg) for arg in args]) - if cache_key in self.request_cache.data.setdefault(func.__name__, {}): - return self.request_cache.data[func.__name__][cache_key] - - result = func(self, *args, **kwargs) - - self.request_cache.data[func.__name__][cache_key] = result - return result - else: - return func(self, *args, **kwargs) - return wrapper - - -def hashvalue(arg): - """ - If arg is an xblock, use its location. otherwise just turn it into a string - """ - if isinstance(arg, XBlock): - return unicode(arg.location) - else: - return unicode(arg) - - -class ModuleStoreWriteBase(ModuleStoreReadBase, ModuleStoreWrite): - ''' - Implement interface functionality that can be shared. - ''' - def __init__(self, contentstore, **kwargs): - super(ModuleStoreWriteBase, self).__init__(contentstore=contentstore, **kwargs) - - # TODO: Don't have a runtime just to generate the appropriate mixin classes (cpennington) - # This is only used by partition_fields_by_scope, which is only needed because - # the split mongo store is used for item creation as well as item persistence - self.mixologist = Mixologist(self.xblock_mixins) - - def partition_fields_by_scope(self, category, fields): - """ - Return dictionary of {scope: {field1: val, ..}..} for the fields of this potential xblock - - :param category: the xblock category - :param fields: the dictionary of {fieldname: value} - """ - result = collections.defaultdict(dict) - if fields is None: - return result - cls = self.mixologist.mix(XBlock.load_class(category, select=prefer_xmodules)) - for field_name, value in fields.iteritems(): - field = getattr(cls, field_name) - result[field.scope][field_name] = value - return result - - def create_course(self, org, course, run, user_id, fields=None, runtime=None, **kwargs): - """ - Creates any necessary other things for the course as a side effect and doesn't return - anything useful. The real subclass should call this before it returns the course. - """ - # clone a default 'about' overview module as well - about_location = self.make_course_key(org, course, run).make_usage_key('about', 'overview') - - about_descriptor = XBlock.load_class('about') - overview_template = about_descriptor.get_template('overview.yaml') - self.create_item( - user_id, - about_location.course_key, - about_location.block_type, - block_id=about_location.block_id, - definition_data={'data': overview_template.get('data')}, - metadata=overview_template.get('metadata'), - runtime=runtime, - continue_version=True, - ) - - def clone_course(self, source_course_id, dest_course_id, user_id, fields=None, **kwargs): - """ - This base method just copies the assets. The lower level impls must do the actual cloning of - content. - """ - # copy the assets - if self.contentstore: - self.contentstore.copy_all_course_assets(source_course_id, dest_course_id) - return dest_course_id - - def delete_course(self, course_key, user_id, **kwargs): - """ - This base method just deletes the assets. The lower level impls must do the actual deleting of - content. - """ - # delete the assets - if self.contentstore: - self.contentstore.delete_all_course_assets(course_key) - super(ModuleStoreWriteBase, self).delete_course(course_key, user_id) - - def _drop_database(self): - """ - A destructive operation to drop the underlying database and close all connections. - Intended to be used by test code for cleanup. - """ - if self.contentstore: - self.contentstore._drop_database() # pylint: disable=protected-access - super(ModuleStoreWriteBase, self)._drop_database() # pylint: disable=protected-access - - def create_child(self, user_id, parent_usage_key, block_type, block_id=None, fields=None, **kwargs): - """ - Creates and saves a new xblock that as a child of the specified block - - Returns the newly created item. - - Args: - user_id: ID of the user creating and saving the xmodule - parent_usage_key: a :class:`~opaque_key.edx.UsageKey` identifing the - block that this item should be parented under - block_type: The type of block to create - block_id: a unique identifier for the new item. If not supplied, - a new identifier will be generated - fields (dict): A dictionary specifying initial values for some or all fields - in the newly created block - """ - item = self.create_item(user_id, parent_usage_key.course_key, block_type, block_id=block_id, fields=fields, **kwargs) - parent = self.get_item(parent_usage_key) - parent.children.append(item.location) - self.update_item(parent, user_id) - def _find_course_assets(self, course_key): """ Base method to override. @@ -916,40 +306,6 @@ class ModuleStoreWriteBase(ModuleStoreReadBase, ModuleStoreWrite): return course_assets, None - def _save_asset_info(self, course_key, asset_metadata, user_id, thumbnail=False): - """ - Base method to over-ride in modulestore. - """ - raise NotImplementedError() - - @contract(course_key='CourseKey', asset_metadata='AssetMetadata') - def save_asset_metadata(self, course_key, asset_metadata, user_id): - """ - Saves the asset metadata for a particular course's asset. - - Arguments: - course_key (CourseKey): course identifier - asset_metadata (AssetMetadata): data about the course asset data - - Returns: - True if metadata save was successful, else False - """ - return self._save_asset_info(course_key, asset_metadata, user_id, thumbnail=False) - - @contract(course_key='CourseKey', asset_thumbnail_metadata='AssetThumbnailMetadata') - def save_asset_thumbnail_metadata(self, course_key, asset_thumbnail_metadata, user_id): - """ - Saves the asset thumbnail metadata for a particular course asset's thumbnail. - - Arguments: - course_key (CourseKey): course identifier - asset_thumbnail_metadata (AssetThumbnailMetadata): data about the course asset thumbnail - - Returns: - True if thumbnail metadata save was successful, else False - """ - return self._save_asset_info(course_key, asset_thumbnail_metadata, user_id, thumbnail=True) - @contract(asset_key='AssetKey') def _find_asset_info(self, asset_key, thumbnail=False, **kwargs): """ @@ -1087,6 +443,45 @@ class ModuleStoreWriteBase(ModuleStoreReadBase, ModuleStoreWrite): """ return self._get_all_asset_metadata(course_key, get_thumbnails=True, **kwargs) + +class ModuleStoreAssetWriteInterface(ModuleStoreAssetInterface): + """ + The write operations for assets and asset metadata + """ + def _save_asset_info(self, course_key, asset_metadata, user_id, thumbnail=False): + """ + Base method to over-ride in modulestore. + """ + raise NotImplementedError() + + @contract(course_key='CourseKey', asset_metadata='AssetMetadata') + def save_asset_metadata(self, course_key, asset_metadata, user_id): + """ + Saves the asset metadata for a particular course's asset. + + Arguments: + course_key (CourseKey): course identifier + asset_metadata (AssetMetadata): data about the course asset data + + Returns: + True if metadata save was successful, else False + """ + return self._save_asset_info(course_key, asset_metadata, user_id, thumbnail=False) + + @contract(course_key='CourseKey', asset_thumbnail_metadata='AssetThumbnailMetadata') + def save_asset_thumbnail_metadata(self, course_key, asset_thumbnail_metadata, user_id): + """ + Saves the asset thumbnail metadata for a particular course asset's thumbnail. + + Arguments: + course_key (CourseKey): course identifier + asset_thumbnail_metadata (AssetThumbnailMetadata): data about the course asset thumbnail + + Returns: + True if thumbnail metadata save was successful, else False + """ + return self._save_asset_info(course_key, asset_thumbnail_metadata, user_id, thumbnail=True) + def set_asset_metadata_attrs(self, asset_key, attrs, user_id): """ Base method to over-ride in modulestore. @@ -1153,6 +548,618 @@ class ModuleStoreWriteBase(ModuleStoreReadBase, ModuleStoreWrite): pass +# pylint: disable=abstract-method +class ModuleStoreRead(ModuleStoreAssetInterface): + """ + An abstract interface for a database backend that stores XModuleDescriptor + instances and extends read-only functionality + """ + + __metaclass__ = ABCMeta + + @abstractmethod + def has_item(self, usage_key): + """ + Returns True if usage_key exists in this ModuleStore. + """ + pass + + @abstractmethod + def get_item(self, usage_key, depth=0, **kwargs): + """ + Returns an XModuleDescriptor instance for the item at location. + + If any segment of the location is None except revision, raises + xmodule.modulestore.exceptions.InsufficientSpecificationError + + If no object is found at that location, raises + xmodule.modulestore.exceptions.ItemNotFoundError + + usage_key: A :class:`.UsageKey` subclass instance + + depth (int): An argument that some module stores may use to prefetch + descendents of the queried modules for more efficient results later + in the request. The depth is counted in the number of calls to + get_children() to cache. None indicates to cache all descendents + """ + pass + + @abstractmethod + def get_course_errors(self, course_key): + """ + Return a list of (msg, exception-or-None) errors that the modulestore + encountered when loading the course at course_id. + + Raises the same exceptions as get_item if the location isn't found or + isn't fully specified. + + Args: + course_key (:class:`.CourseKey`): The course to check for errors + """ + pass + + @abstractmethod + def get_items(self, course_id, qualifiers=None, **kwargs): + """ + Returns a list of XModuleDescriptor instances for the items + that match location. Any element of location that is None is treated + as a wildcard that matches any value + + location: Something that can be passed to Location + """ + pass + + def _block_matches(self, fields_or_xblock, 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) + + 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 + else: + fields = fields_or_xblock + is_xblock = False + + def _is_set_on(key): + """ + Is this key set in fields? (return tuple of boolean and value). A helper which can + handle fields either being the json doc or xblock fields. Is inner function to restrict + use and to access local vars. + """ + 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) + else: + return True, field + + for key, criteria in qualifiers.iteritems(): + is_set, value = _is_set_on(key) + if not is_set: + return False + if not self._value_matches(value, criteria): + return False + 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 + If the criteria is a regex, does the target match it? + 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 + return criteria.search(target) is not None + elif callable(criteria): + return criteria(target) + elif isinstance(criteria, dict) and '$in' in criteria: + # note isn't handling any other things in the dict other than in + return any(self._value_matches(target, test_val) for test_val in criteria['$in']) + elif isinstance(criteria, dict) and '$nin' in criteria: + # note isn't handling any other things in the dict other than nin + return not any(self._value_matches(target, test_val) for test_val in criteria['$nin']) + else: + return criteria == target + + @abstractmethod + def make_course_key(self, org, course, run): + """ + Return a valid :class:`~opaque_keys.edx.keys.CourseKey` for this modulestore + that matches the supplied `org`, `course`, and `run`. + + This key may represent a course that doesn't exist in this modulestore. + """ + pass + + @abstractmethod + def get_courses(self, **kwargs): + ''' + Returns a list containing the top level XModuleDescriptors of the courses + in this modulestore. + ''' + pass + + @abstractmethod + def get_course(self, course_id, depth=0, **kwargs): + ''' + Look for a specific course by its id (:class:`CourseKey`). + Returns the course descriptor, or None if not found. + ''' + pass + + @abstractmethod + def has_course(self, course_id, ignore_case=False, **kwargs): + ''' + Look for a specific course id. Returns whether it exists. + Args: + course_id (CourseKey): + ignore_case (boolean): some modulestores are case-insensitive. Use this flag + to search for whether a potentially conflicting course exists in that case. + ''' + pass + + @abstractmethod + def get_parent_location(self, location, **kwargs): + ''' + Find the location that is the parent of this location in this + course. Needed for path_to_location(). + ''' + pass + + @abstractmethod + def get_orphans(self, course_key, **kwargs): + """ + Get all of the xblocks in the given course which have no parents and are not of types which are + usually orphaned. NOTE: may include xblocks which still have references via xblocks which don't + use children to point to their dependents. + """ + pass + + @abstractmethod + def get_errored_courses(self): + """ + Return a dictionary of course_dir -> [(msg, exception_str)], for each + course_dir where course loading failed. + """ + pass + + @abstractmethod + def get_modulestore_type(self, course_id): + """ + Returns a type which identifies which modulestore is servicing the given + course_id. The return can be either "xml" (for XML based courses) or "mongo" for MongoDB backed courses + """ + pass + + @abstractmethod + def get_courses_for_wiki(self, wiki_slug, **kwargs): + """ + Return the list of courses which use this wiki_slug + :param wiki_slug: the course wiki root slug + :return: list of course keys + """ + pass + + @abstractmethod + def has_published_version(self, xblock): + """ + Returns true if this xblock exists in the published course regardless of whether it's up to date + """ + pass + + @abstractmethod + def close_connections(self): + """ + Closes any open connections to the underlying databases + """ + pass + + @contextmanager + def bulk_operations(self, course_id): + """ + A context manager for notifying the store of bulk operations. This affects only the current thread. + """ + yield + + def ensure_indexes(self): + """ + Ensure that all appropriate indexes are created that are needed by this modulestore, or raise + an exception if unable to. + + This method is intended for use by tests and administrative commands, and not + to be run during server startup. + """ + pass + + +# pylint: disable=abstract-method +class ModuleStoreWrite(ModuleStoreRead, ModuleStoreAssetWriteInterface): + """ + An abstract interface for a database backend that stores XModuleDescriptor + instances and extends both read and write functionality + """ + + __metaclass__ = ABCMeta + + @abstractmethod + def update_item(self, xblock, user_id, allow_not_found=False, force=False, **kwargs): + """ + Update the given xblock's persisted repr. Pass the user's unique id which the persistent store + should save with the update if it has that ability. + + :param allow_not_found: whether this method should raise an exception if the given xblock + has not been persisted before. + :param force: fork the structure and don't update the course draftVersion if there's a version + conflict (only applicable to version tracking and conflict detecting persistence stores) + + :raises VersionConflictError: if org, course, run, and version_guid given and the current + version head != version_guid and force is not True. (only applicable to version tracking stores) + """ + pass + + @abstractmethod + def delete_item(self, location, user_id, **kwargs): + """ + Delete an item and its subtree from persistence. Remove the item from any parents (Note, does not + affect parents from other branches or logical branches; thus, in old mongo, deleting something + whose parent cannot be draft, deletes it from both but deleting a component under a draft vertical + only deletes it from the draft. + + Pass the user's unique id which the persistent store + should save with the update if it has that ability. + + :param force: fork the structure and don't update the course draftVersion if there's a version + conflict (only applicable to version tracking and conflict detecting persistence stores) + + :raises VersionConflictError: if org, course, run, and version_guid given and the current + version head != version_guid and force is not True. (only applicable to version tracking stores) + """ + pass + + @abstractmethod + def create_course(self, org, course, run, user_id, fields=None, **kwargs): + """ + Creates and returns the course. + + Args: + org (str): the organization that owns the course + course (str): the name of the course + run (str): the name of the run + user_id: id of the user creating the course + fields (dict): Fields to set on the course at initialization + kwargs: Any optional arguments understood by a subset of modulestores to customize instantiation + + Returns: a CourseDescriptor + """ + pass + + @abstractmethod + def create_item(self, user_id, course_key, block_type, block_id=None, fields=None, **kwargs): + """ + Creates and saves a new item in a course. + + Returns the newly created item. + + Args: + user_id: ID of the user creating and saving the xmodule + course_key: A :class:`~opaque_keys.edx.CourseKey` identifying which course to create + this item in + block_type: The type of block to create + block_id: a unique identifier for the new item. If not supplied, + a new identifier will be generated + fields (dict): A dictionary specifying initial values for some or all fields + in the newly created block + """ + pass + + @abstractmethod + def clone_course(self, source_course_id, dest_course_id, user_id, fields=None): + """ + Sets up source_course_id to point a course with the same content as the desct_course_id. This + operation may be cheap or expensive. It may have to copy all assets and all xblock content or + merely setup new pointers. + + Backward compatibility: this method used to require in some modulestores that dest_course_id + pointed to an empty but already created course. Implementers should support this or should + enable creating the course from scratch. + + Raises: + ItemNotFoundError: if the source course doesn't exist (or any of its xblocks aren't found) + DuplicateItemError: if the destination course already exists (with content in some cases) + """ + pass + + @abstractmethod + def delete_course(self, course_key, user_id, **kwargs): + """ + Deletes the course. It may be a soft or hard delete. It may or may not remove the xblock definitions + depending on the persistence layer and how tightly bound the xblocks are to the course. + + Args: + course_key (CourseKey): which course to delete + user_id: id of the user deleting the course + """ + pass + + @abstractmethod + def _drop_database(self): + """ + A destructive operation to drop the underlying database and close all connections. + Intended to be used by test code for cleanup. + """ + pass + + +# pylint: disable=abstract-method +class ModuleStoreReadBase(BulkOperationsMixin, ModuleStoreRead): + ''' + Implement interface functionality that can be shared. + ''' + + # pylint: disable=invalid-name + def __init__( + self, + contentstore=None, + doc_store_config=None, # ignore if passed up + metadata_inheritance_cache_subsystem=None, request_cache=None, + xblock_mixins=(), xblock_select=None, + # temporary parms to enable backward compatibility. remove once all envs migrated + db=None, collection=None, host=None, port=None, tz_aware=True, user=None, password=None, + # allow lower level init args to pass harmlessly + ** kwargs + ): + ''' + Set up the error-tracking logic. + ''' + super(ModuleStoreReadBase, self).__init__(**kwargs) + self._course_errors = defaultdict(make_error_tracker) # location -> ErrorLog + # pylint: disable=fixme + # TODO move the inheritance_cache_subsystem to classes which use it + self.metadata_inheritance_cache_subsystem = metadata_inheritance_cache_subsystem + self.request_cache = request_cache + self.xblock_mixins = xblock_mixins + self.xblock_select = xblock_select + self.contentstore = contentstore + + def get_course_errors(self, course_key): + """ + Return list of errors for this :class:`.CourseKey`, if any. Raise the same + errors as get_item if course_key isn't present. + """ + # check that item is present and raise the promised exceptions if needed + # pylint: disable=fixme + # TODO (vshnayder): post-launch, make errors properties of items + # self.get_item(location) + assert(isinstance(course_key, CourseKey)) + return self._course_errors[course_key].errors + + def get_errored_courses(self): + """ + Returns an empty dict. + + It is up to subclasses to extend this method if the concept + of errored courses makes sense for their implementation. + """ + return {} + + def get_course(self, course_id, depth=0, **kwargs): + """ + See ModuleStoreRead.get_course + + Default impl--linear search through course list + """ + assert(isinstance(course_id, CourseKey)) + for course in self.get_courses(**kwargs): + if course.id == course_id: + return course + return None + + def has_course(self, course_id, ignore_case=False, **kwargs): + """ + Returns the course_id of the course if it was found, else None + Args: + course_id (CourseKey): + ignore_case (boolean): some modulestores are case-insensitive. Use this flag + to search for whether a potentially conflicting course exists in that case. + """ + # linear search through list + assert(isinstance(course_id, CourseKey)) + if ignore_case: + return next( + ( + c.id for c in self.get_courses() + if c.id.org.lower() == course_id.org.lower() and + c.id.course.lower() == course_id.course.lower() and + c.id.run.lower() == course_id.run.lower() + ), + None + ) + else: + return next( + (c.id for c in self.get_courses() if c.id == course_id), + None + ) + + def has_published_version(self, xblock): + """ + Returns True since this is a read-only store. + """ + return True + + def heartbeat(self): + """ + Is this modulestore ready? + """ + # default is to say yes by not raising an exception + return {'default_impl': True} + + def close_connections(self): + """ + Closes any open connections to the underlying databases + """ + if self.contentstore: + self.contentstore.close_connections() + super(ModuleStoreReadBase, self).close_connections() + + @contextmanager + def default_store(self, store_type): + """ + A context manager for temporarily changing the default store + """ + if self.get_modulestore_type(None) != store_type: + raise ValueError(u"Cannot set default store to type {}".format(store_type)) + yield + + @staticmethod + def memoize_request_cache(func): + """ + Memoize a function call results on the request_cache if there's one. Creates the cache key by + joining the unicode of all the args with &; so, if your arg may use the default &, it may + have false hits + """ + @functools.wraps(func) + def wrapper(self, *args, **kwargs): + """ + Wraps a method to memoize results. + """ + if self.request_cache: + cache_key = '&'.join([hashvalue(arg) for arg in args]) + if cache_key in self.request_cache.data.setdefault(func.__name__, {}): + return self.request_cache.data[func.__name__][cache_key] + + result = func(self, *args, **kwargs) + + self.request_cache.data[func.__name__][cache_key] = result + return result + else: + return func(self, *args, **kwargs) + return wrapper + + +def hashvalue(arg): + """ + If arg is an xblock, use its location. otherwise just turn it into a string + """ + if isinstance(arg, XBlock): + return unicode(arg.location) + else: + return unicode(arg) + + +# pylint: disable=abstract-method +class ModuleStoreWriteBase(ModuleStoreReadBase, ModuleStoreWrite): + ''' + Implement interface functionality that can be shared. + ''' + def __init__(self, contentstore, **kwargs): + super(ModuleStoreWriteBase, self).__init__(contentstore=contentstore, **kwargs) + self.mixologist = Mixologist(self.xblock_mixins) + + def partition_fields_by_scope(self, category, fields): + """ + Return dictionary of {scope: {field1: val, ..}..} for the fields of this potential xblock + + :param category: the xblock category + :param fields: the dictionary of {fieldname: value} + """ + result = collections.defaultdict(dict) + if fields is None: + return result + cls = self.mixologist.mix(XBlock.load_class(category, select=prefer_xmodules)) + for field_name, value in fields.iteritems(): + field = getattr(cls, field_name) + result[field.scope][field_name] = value + return result + + def create_course(self, org, course, run, user_id, fields=None, runtime=None, **kwargs): + """ + Creates any necessary other things for the course as a side effect and doesn't return + anything useful. The real subclass should call this before it returns the course. + """ + # clone a default 'about' overview module as well + about_location = self.make_course_key(org, course, run).make_usage_key('about', 'overview') + + about_descriptor = XBlock.load_class('about') + overview_template = about_descriptor.get_template('overview.yaml') + self.create_item( + user_id, + about_location.course_key, + about_location.block_type, + block_id=about_location.block_id, + definition_data={'data': overview_template.get('data')}, + metadata=overview_template.get('metadata'), + runtime=runtime, + continue_version=True, + ) + + def clone_course(self, source_course_id, dest_course_id, user_id, fields=None, **kwargs): + """ + This base method just copies the assets. The lower level impls must do the actual cloning of + content. + """ + # copy the assets + if self.contentstore: + self.contentstore.copy_all_course_assets(source_course_id, dest_course_id) + return dest_course_id + + def delete_course(self, course_key, user_id, **kwargs): + """ + This base method just deletes the assets. The lower level impls must do the actual deleting of + content. + """ + # delete the assets + if self.contentstore: + self.contentstore.delete_all_course_assets(course_key) + super(ModuleStoreWriteBase, self).delete_course(course_key, user_id) + + def _drop_database(self): + """ + A destructive operation to drop the underlying database and close all connections. + Intended to be used by test code for cleanup. + """ + if self.contentstore: + self.contentstore._drop_database() # pylint: disable=protected-access + super(ModuleStoreWriteBase, self)._drop_database() # pylint: disable=protected-access + + def create_child(self, user_id, parent_usage_key, block_type, block_id=None, fields=None, **kwargs): + """ + Creates and saves a new xblock that as a child of the specified block + + Returns the newly created item. + + Args: + user_id: ID of the user creating and saving the xmodule + parent_usage_key: a :class:`~opaque_key.edx.UsageKey` identifing the + block that this item should be parented under + block_type: The type of block to create + block_id: a unique identifier for the new item. If not supplied, + a new identifier will be generated + fields (dict): A dictionary specifying initial values for some or all fields + in the newly created block + """ + item = self.create_item(user_id, parent_usage_key.course_key, block_type, block_id=block_id, fields=fields, **kwargs) + parent = self.get_item(parent_usage_key) + parent.children.append(item.location) + self.update_item(parent, user_id) + + def only_xmodules(identifier, entry_points): """Only use entry_points that are supplied by the xmodule package""" from_xmodule = [entry_point for entry_point in entry_points if entry_point.dist.key == 'xmodule'] diff --git a/common/lib/xmodule/xmodule/modulestore/tests/test_assetstore.py b/common/lib/xmodule/xmodule/modulestore/tests/test_assetstore.py index 79f3b84c11..1b0d83c63a 100644 --- a/common/lib/xmodule/xmodule/modulestore/tests/test_assetstore.py +++ b/common/lib/xmodule/xmodule/modulestore/tests/test_assetstore.py @@ -12,7 +12,7 @@ from xmodule.modulestore import ModuleStoreEnum from xmodule.modulestore.tests.factories import CourseFactory from xmodule.modulestore.tests.test_cross_modulestore_import_export import ( - MODULESTORE_SETUPS, MongoContentstoreBuilder, + MODULESTORE_SETUPS, MongoContentstoreBuilder, XmlModulestoreBuilder, MixedModulestoreBuilder ) @@ -392,3 +392,21 @@ class TestMongoAssetMetadataStorage(unittest.TestCase): def test_copy_all_assets(self): pass + + @ddt.data(XmlModulestoreBuilder(), MixedModulestoreBuilder([('xml', XmlModulestoreBuilder())])) + def test_xml_not_yet_implemented(self, storebuilder): + """ + Test coverage which shows that for now xml read operations are not implemented + """ + with storebuilder.build(None) as store: + course_key = store.make_course_key("org", "course", "run") + asset_key = course_key.make_asset_key('asset', 'foo.jpg') + for method in ['_find_asset_info', 'find_asset_metadata', 'find_asset_thumbnail_metadata']: + with self.assertRaises(NotImplementedError): + getattr(store, method)(asset_key) + with self.assertRaises(NotImplementedError): + # pylint: disable=protected-access + store._find_course_asset(course_key, asset_key.block_id) + for method in ['_get_all_asset_metadata', 'get_all_asset_metadata', 'get_all_asset_thumbnail_metadata']: + with self.assertRaises(NotImplementedError): + getattr(store, method)(course_key) diff --git a/common/lib/xmodule/xmodule/modulestore/tests/test_cross_modulestore_import_export.py b/common/lib/xmodule/xmodule/modulestore/tests/test_cross_modulestore_import_export.py index d08bb518fa..be60c10b95 100644 --- a/common/lib/xmodule/xmodule/modulestore/tests/test_cross_modulestore_import_export.py +++ b/common/lib/xmodule/xmodule/modulestore/tests/test_cross_modulestore_import_export.py @@ -17,6 +17,7 @@ import random from contextlib import contextmanager, nested from shutil import rmtree from tempfile import mkdtemp +from path import path from xmodule.tests import CourseComparisonTest @@ -30,13 +31,14 @@ from xmodule.modulestore.split_mongo.split_draft import DraftVersioningModuleSto from xmodule.modulestore.tests.mongo_connection import MONGO_PORT_NUM, MONGO_HOST from xmodule.modulestore.inheritance import InheritanceMixin from xmodule.x_module import XModuleMixin +from xmodule.modulestore.xml import XMLModuleStore COMMON_DOCSTORE_CONFIG = { 'host': MONGO_HOST, 'port': MONGO_PORT_NUM, } - +DATA_DIR = path(__file__).dirname().parent.parent.parent.parent.parent / "test" / "data" XBLOCK_MIXINS = (InheritanceMixin, XModuleMixin) @@ -163,6 +165,30 @@ class VersioningModulestoreBuilder(object): return 'SplitModulestoreBuilder()' +class XmlModulestoreBuilder(object): + """ + A builder class for a XMLModuleStore. + """ + # pylint: disable=unused-argument + @contextmanager + def build(self, contentstore=None, course_ids=None): + """ + A contextmanager that returns an isolated xml modulestore + + Args: + contentstore: The contentstore that this modulestore should use to store + all of its assets. + """ + modulestore = XMLModuleStore( + DATA_DIR, + course_ids=course_ids, + default_class='xmodule.hidden_module.HiddenDescriptor', + xblock_mixins=XBLOCK_MIXINS, + ) + + yield modulestore + + class MixedModulestoreBuilder(object): """ A builder class for a MixedModuleStore. diff --git a/common/lib/xmodule/xmodule/modulestore/xml.py b/common/lib/xmodule/xmodule/modulestore/xml.py index aade361934..c0bb929bce 100644 --- a/common/lib/xmodule/xmodule/modulestore/xml.py +++ b/common/lib/xmodule/xmodule/modulestore/xml.py @@ -846,3 +846,52 @@ class XMLModuleStore(ModuleStoreReadBase): if branch_setting != ModuleStoreEnum.Branch.published_only: raise ValueError(u"Cannot set branch setting to {} on a ReadOnly store".format(branch_setting)) yield + + def _find_course_asset(self, course_key, filename, get_thumbnail=False): + """ + For now this is not implemented, but others should feel free to implement using the asset.json + which export produces. + """ + raise NotImplementedError() + + def _find_asset_info(self, asset_key, thumbnail=False, **kwargs): + """ + For now this is not implemented, but others should feel free to implement using the asset.json + which export produces. + """ + raise NotImplementedError() + + def find_asset_metadata(self, asset_key, **kwargs): + """ + For now this is not implemented, but others should feel free to implement using the asset.json + which export produces. + """ + raise NotImplementedError() + + def find_asset_thumbnail_metadata(self, asset_key, **kwargs): + """ + For now this is not implemented, but others should feel free to implement using the asset.json + which export produces. + """ + raise NotImplementedError() + + def _get_all_asset_metadata(self, course_key, start=0, maxresults=-1, sort=None, get_thumbnails=False, **kwargs): + """ + For now this is not implemented, but others should feel free to implement using the asset.json + which export produces. + """ + raise NotImplementedError() + + def get_all_asset_metadata(self, course_key, start=0, maxresults=-1, sort=None, **kwargs): + """ + For now this is not implemented, but others should feel free to implement using the asset.json + which export produces. + """ + raise NotImplementedError() + + def get_all_asset_thumbnail_metadata(self, course_key, **kwargs): + """ + For now this is not implemented, but others should feel free to implement using the asset.json + which export produces. + """ + raise NotImplementedError()