diff --git a/cms/djangoapps/contentstore/views/tests/test_assets.py b/cms/djangoapps/contentstore/views/tests/test_assets.py index daa7450f2b..22f26fd16f 100644 --- a/cms/djangoapps/contentstore/views/tests/test_assets.py +++ b/cms/djangoapps/contentstore/views/tests/test_assets.py @@ -309,13 +309,14 @@ class DownloadTestCase(AssetsTestCase): def test_metadata_found_in_modulestore(self): # Insert asset metadata into the modulestore (with no accompanying asset). asset_key = self.course.id.make_asset_key(AssetMetadata.GENERAL_ASSET_TYPE, 'pic1.jpg') - asset_md = AssetMetadata(asset_key, { - 'internal_name': 'EKMND332DDBK', - 'basename': 'pix/archive', - 'locked': False, - 'curr_version': '14', - 'prev_version': '13' - }) + asset_md = AssetMetadata( + asset_key, + internal_name='EKMND332DDBK', + pathname='pix/archive', + locked=False, + curr_version='14', + prev_version='13', + ) modulestore().save_asset_metadata(asset_md, 15) # Get the asset metadata and have it be found in the modulestore. # Currently, no asset metadata should be found in the modulestore. The code is not yet storing it there. diff --git a/common/lib/xmodule/xmodule/modulestore/mongo/base.py b/common/lib/xmodule/xmodule/modulestore/mongo/base.py index f7cc509de8..b49391f9f8 100644 --- a/common/lib/xmodule/xmodule/modulestore/mongo/base.py +++ b/common/lib/xmodule/xmodule/modulestore/mongo/base.py @@ -120,7 +120,10 @@ class MongoKeyValueStore(InheritanceKeyValueStore): elif key.scope == Scope.content: return self._data[key.field_name] else: - raise InvalidScopeError(key) + raise InvalidScopeError( + key, + (Scope.children, Scope.parent, Scope.settings, Scope.content), + ) def set(self, key, value): if key.scope == Scope.children: @@ -130,7 +133,10 @@ class MongoKeyValueStore(InheritanceKeyValueStore): elif key.scope == Scope.content: self._data[key.field_name] = value else: - raise InvalidScopeError(key) + raise InvalidScopeError( + key, + (Scope.children, Scope.settings, Scope.content), + ) def delete(self, key): if key.scope == Scope.children: @@ -142,7 +148,10 @@ class MongoKeyValueStore(InheritanceKeyValueStore): if key.field_name in self._data: del self._data[key.field_name] else: - raise InvalidScopeError(key) + raise InvalidScopeError( + key, + (Scope.children, Scope.settings, Scope.content), + ) def has(self, key): if key.scope in (Scope.children, Scope.parent): diff --git a/common/lib/xmodule/xmodule/modulestore/split_mongo/split_mongo_kvs.py b/common/lib/xmodule/xmodule/modulestore/split_mongo/split_mongo_kvs.py index 2124732242..8720b7c3d9 100644 --- a/common/lib/xmodule/xmodule/modulestore/split_mongo/split_mongo_kvs.py +++ b/common/lib/xmodule/xmodule/modulestore/split_mongo/split_mongo_kvs.py @@ -18,6 +18,8 @@ class SplitMongoKVS(InheritanceKeyValueStore): known to the MongoModuleStore (data, children, and metadata) """ + VALID_SCOPES = (Scope.parent, Scope.children, Scope.settings, Scope.content) + @contract(parent="BlockUsageLocator | None") def __init__(self, definition, initial_values, default_values, parent, field_decorator=None): """ @@ -59,7 +61,7 @@ class SplitMongoKVS(InheritanceKeyValueStore): else: raise KeyError() else: - raise InvalidScopeError(key) + raise InvalidScopeError(key, self.VALID_SCOPES) if key.field_name in self._fields: field_value = self._fields[key.field_name] @@ -71,8 +73,8 @@ class SplitMongoKVS(InheritanceKeyValueStore): def set(self, key, value): # handle any special cases - if key.scope not in [Scope.children, Scope.settings, Scope.content]: - raise InvalidScopeError(key) + if key.scope not in self.VALID_SCOPES: + raise InvalidScopeError(key, self.VALID_SCOPES) if key.scope == Scope.content: self._load_definition() @@ -90,8 +92,8 @@ class SplitMongoKVS(InheritanceKeyValueStore): def delete(self, key): # handle any special cases - if key.scope not in [Scope.children, Scope.settings, Scope.content]: - raise InvalidScopeError(key) + if key.scope not in self.VALID_SCOPES: + raise InvalidScopeError(key, self.VALID_SCOPES) if key.scope == Scope.content: self._load_definition() diff --git a/lms/djangoapps/courseware/model_data.py b/lms/djangoapps/courseware/model_data.py index 8a3bcb7007..c47ee65b41 100644 --- a/lms/djangoapps/courseware/model_data.py +++ b/lms/djangoapps/courseware/model_data.py @@ -1,10 +1,29 @@ """ -Classes to provide the LMS runtime data storage to XBlocks +Classes to provide the LMS runtime data storage to XBlocks. + +:class:`DjangoKeyValueStore`: An XBlock :class:`~KeyValueStore` which + stores a subset of xblocks scopes as Django ORM objects. It wraps + :class:`~FieldDataCache` to provide an XBlock-friendly interface. + + +:class:`FieldDataCache`: A object which provides a read-through prefetch cache + of data to support XBlock fields within a limited set of scopes. + +The remaining classes in this module provide read-through prefetch cache implementations +for specific scopes. The individual classes provide the knowledge of what are the essential +pieces of information for each scope, and thus how to cache, prefetch, and create new field data +entries. + +UserStateCache: A cache for Scope.user_state +UserStateSummaryCache: A cache for Scope.user_state_summary +PreferencesCache: A cache for Scope.preferences +UserInfoCache: A cache for Scope.user_info +DjangoOrmFieldCache: A base-class for single-row-per-field caches. """ import json +from abc import abstractmethod, ABCMeta from collections import defaultdict -from itertools import chain from .models import ( StudentModule, XModuleUserStateSummaryField, @@ -12,9 +31,10 @@ from .models import ( XModuleStudentInfoField ) import logging -from opaque_keys.edx.keys import CourseKey +from opaque_keys.edx.keys import CourseKey, UsageKey from opaque_keys.edx.block_types import BlockTypeKeyV1 from opaque_keys.edx.asides import AsideUsageKeyV1 +from contracts import contract, new_contract from django.db import DatabaseError @@ -23,6 +43,7 @@ from xblock.exceptions import KeyValueMultiSaveError, InvalidScopeError from xblock.fields import Scope, UserScope from xmodule.modulestore.django import modulestore from xblock.core import XBlockAside +from courseware.user_state_client import DjangoXBlockUserStateClient log = logging.getLogger(__name__) @@ -34,21 +55,655 @@ class InvalidWriteError(Exception): """ -def chunks(items, chunk_size): +def _all_usage_keys(descriptors, aside_types): """ - Yields the values from items in chunks of size chunk_size + Return a set of all usage_ids for the `descriptors` and for + as all asides in `aside_types` for those descriptors. """ - items = list(items) - return (items[i:i + chunk_size] for i in xrange(0, len(items), chunk_size)) + usage_ids = set() + for descriptor in descriptors: + usage_ids.add(descriptor.scope_ids.usage_id) + + for aside_type in aside_types: + usage_ids.add(AsideUsageKeyV1(descriptor.scope_ids.usage_id, aside_type)) + + return usage_ids + + +def _all_block_types(descriptors, aside_types): + """ + Return a set of all block_types for the supplied `descriptors` and for + the asides types in `aside_types` associated with those descriptors. + """ + block_types = set() + for descriptor in descriptors: + block_types.add(BlockTypeKeyV1(descriptor.entry_point, descriptor.scope_ids.block_type)) + + for aside_type in aside_types: + block_types.add(BlockTypeKeyV1(XBlockAside.entry_point, aside_type)) + + return block_types + + +class DjangoKeyValueStore(KeyValueStore): + """ + This KeyValueStore will read and write data in the following scopes to django models + Scope.user_state_summary + Scope.user_state + Scope.preferences + Scope.user_info + + Access to any other scopes will raise an InvalidScopeError + + Data for Scope.user_state is stored as StudentModule objects via the django orm. + + Data for the other scopes is stored in individual objects that are named for the + scope involved and have the field name as a key + + If the key isn't found in the expected table during a read or a delete, then a KeyError will be raised + """ + + _allowed_scopes = ( + Scope.user_state_summary, + Scope.user_state, + Scope.preferences, + Scope.user_info, + ) + + def __init__(self, field_data_cache): + self._field_data_cache = field_data_cache + + def get(self, key): + self._raise_unless_scope_is_allowed(key) + return self._field_data_cache.get(key) + + def set(self, key, value): + """ + Set a single value in the KeyValueStore + """ + self.set_many({key: value}) + + def set_many(self, kv_dict): + """ + Provide a bulk save mechanism. + + `kv_dict`: A dictionary of dirty fields that maps + xblock.KvsFieldData._key : value + + """ + for key in kv_dict: + # Check key for validity + self._raise_unless_scope_is_allowed(key) + + self._field_data_cache.set_many(kv_dict) + + def delete(self, key): + self._raise_unless_scope_is_allowed(key) + self._field_data_cache.delete(key) + + def has(self, key): + self._raise_unless_scope_is_allowed(key) + return self._field_data_cache.has(key) + + def _raise_unless_scope_is_allowed(self, key): + """Raise an InvalidScopeError if key.scope is not in self._allowed_scopes.""" + if key.scope not in self._allowed_scopes: + raise InvalidScopeError(key, self._allowed_scopes) + + +new_contract("DjangoKeyValueStore", DjangoKeyValueStore) +new_contract("DjangoKeyValueStore_Key", DjangoKeyValueStore.Key) + + +class DjangoOrmFieldCache(object): + """ + Baseclass for Scope-specific field cache objects that are based on + single-row-per-field Django ORM objects. + """ + __metaclass__ = ABCMeta + + def __init__(self): + self._cache = {} + + def cache_fields(self, fields, xblocks, aside_types): + """ + Load all fields specified by ``fields`` for the supplied ``xblocks`` + and ``aside_types`` into this cache. + + Arguments: + fields (list of str): Field names to cache. + xblocks (list of :class:`XBlock`): XBlocks to cache fields for. + aside_types (list of str): Aside types to cache fields for. + """ + for field_object in self._read_objects(fields, xblocks, aside_types): + self._cache[self._cache_key_for_field_object(field_object)] = field_object + + @contract(kvs_key=DjangoKeyValueStore.Key) + def get(self, kvs_key): + """ + Return the django model object specified by `kvs_key` from + the cache. + + Arguments: + kvs_key (`DjangoKeyValueStore.Key`): The field value to delete + + Returns: A django orm object from the cache + """ + cache_key = self._cache_key_for_kvs_key(kvs_key) + if cache_key not in self._cache: + raise KeyError(kvs_key.field_name) + + field_object = self._cache[cache_key] + + return json.loads(field_object.value) + + @contract(kvs_key=DjangoKeyValueStore.Key) + def set(self, kvs_key, value): + """ + Set the specified `kvs_key` to the field value `value`. + + Arguments: + kvs_key (`DjangoKeyValueStore.Key`): The field value to delete + value: The field value to store + """ + self.set_many({kvs_key: value}) + + @contract(kv_dict="dict(DjangoKeyValueStore_Key: *)") + def set_many(self, kv_dict): + """ + Set the specified fields to the supplied values. + + Arguments: + kv_dict (dict): A dictionary mapping :class:`~DjangoKeyValueStore.Key` + objects to values to set. + """ + saved_fields = [] + for kvs_key, value in sorted(kv_dict.items()): + cache_key = self._cache_key_for_kvs_key(kvs_key) + field_object = self._cache.get(cache_key) + + try: + serialized_value = json.dumps(value) + # It is safe to force an insert or an update, because + # a) we should have retrieved the object as part of the + # prefetch step, so if it isn't in our cache, it doesn't exist yet. + # b) no other code should be modifying these models out of band of + # this cache. + if field_object is None: + field_object = self._create_object(kvs_key, serialized_value) + field_object.save(force_insert=True) + self._cache[cache_key] = field_object + else: + field_object.value = serialized_value + field_object.save(force_update=True) + + except DatabaseError: + log.exception("Saving field %r failed", kvs_key.field_name) + raise KeyValueMultiSaveError(saved_fields) + + finally: + saved_fields.append(kvs_key.field_name) + + @contract(kvs_key=DjangoKeyValueStore.Key) + def delete(self, kvs_key): + """ + Delete the value specified by `kvs_key`. + + Arguments: + kvs_key (`DjangoKeyValueStore.Key`): The field value to delete + + Raises: KeyError if key isn't found in the cache + """ + + cache_key = self._cache_key_for_kvs_key(kvs_key) + field_object = self._cache.get(cache_key) + if field_object is None: + raise KeyError(kvs_key.field_name) + + field_object.delete() + del self._cache[cache_key] + + @contract(kvs_key=DjangoKeyValueStore.Key, returns=bool) + def has(self, kvs_key): + """ + Return whether the specified `kvs_key` is set. + + Arguments: + kvs_key (`DjangoKeyValueStore.Key`): The field value to delete + + Returns: bool + """ + return self._cache_key_for_kvs_key(kvs_key) in self._cache + + @contract(kvs_key=DjangoKeyValueStore.Key, returns="datetime|None") + def last_modified(self, kvs_key): + """ + Return when the supplied field was changed. + + Arguments: + kvs_key (`DjangoKeyValueStore.Key`): The field value to delete + + Returns: datetime if there was a modified date, or None otherwise + """ + field_object = self._cache.get(self._cache_key_for_kvs_key(kvs_key)) + + if field_object is None: + return None + else: + return field_object.modified + + def __len__(self): + return len(self._cache) + + @abstractmethod + def _create_object(self, kvs_key, value): + """ + Create a new object to add to the cache (which should record + the specified field ``value`` for the field identified by + ``kvs_key``). + + Arguments: + kvs_key (:class:`DjangoKeyValueStore.Key`): Which field to create an entry for + value: What value to record in the field + """ + raise NotImplementedError() + + @abstractmethod + def _read_objects(self, fields, xblocks, aside_types): + """ + Return an iterator for all objects stored in the underlying datastore + for the ``fields`` on the ``xblocks`` and the ``aside_types`` associated + with them. + + Arguments: + fields (list of str): Field names to return values for + xblocks (list of :class:`~XBlock`): XBlocks to load fields for + aside_types (list of str): Asides to load field for (which annotate the supplied + xblocks). + """ + raise NotImplementedError() + + @abstractmethod + def _cache_key_for_field_object(self, field_object): + """ + Return the key used in this DjangoOrmFieldCache to store the specified field_object. + + Arguments: + field_object: A Django model instance that stores the data for fields in this cache + """ + raise NotImplementedError() + + @abstractmethod + def _cache_key_for_kvs_key(self, key): + """ + Return the key used in this DjangoOrmFieldCache for the specified KeyValueStore key. + + Arguments: + key (:class:`~DjangoKeyValueStore.Key`): The key representing the cached field + """ + raise NotImplementedError() + + +class UserStateCache(object): + """ + Cache for Scope.user_state xblock field data. + """ + def __init__(self, user, course_id): + self._cache = defaultdict(dict) + self.course_id = course_id + self.user = user + self._client = DjangoXBlockUserStateClient(self.user) + + def cache_fields(self, fields, xblocks, aside_types): # pylint: disable=unused-argument + """ + Load all fields specified by ``fields`` for the supplied ``xblocks`` + and ``aside_types`` into this cache. + + Arguments: + fields (list of str): Field names to cache. + xblocks (list of :class:`XBlock`): XBlocks to cache fields for. + aside_types (list of str): Aside types to cache fields for. + """ + block_field_state = self._client.get_many( + self.user.username, + _all_usage_keys(xblocks, aside_types), + ) + for usage_key, field_state in block_field_state: + self._cache[usage_key] = field_state + + @contract(kvs_key=DjangoKeyValueStore.Key) + def set(self, kvs_key, value): + """ + Set the specified `kvs_key` to the field value `value`. + + Arguments: + kvs_key (`DjangoKeyValueStore.Key`): The field value to delete + value: The field value to store + """ + self.set_many({kvs_key: value}) + + @contract(kvs_key=DjangoKeyValueStore.Key, returns="datetime|None") + def last_modified(self, kvs_key): + """ + Return when the supplied field was changed. + + Arguments: + kvs_key (`DjangoKeyValueStore.Key`): The key representing the cached field + + Returns: datetime if there was a modified date, or None otherwise + """ + return self._client.get_mod_date( + self.user.username, + kvs_key.block_scope_id, + fields=[kvs_key.field_name], + ).get(kvs_key.field_name) + + @contract(kv_dict="dict(DjangoKeyValueStore_Key: *)") + def set_many(self, kv_dict): + """ + Set the specified fields to the supplied values. + + Arguments: + kv_dict (dict): A dictionary mapping :class:`~DjangoKeyValueStore.Key` + objects to values to set. + """ + pending_updates = defaultdict(dict) + for kvs_key, value in kv_dict.items(): + cache_key = self._cache_key_for_kvs_key(kvs_key) + + pending_updates[cache_key][kvs_key.field_name] = value + + try: + self._client.set_many( + self.user.username, + pending_updates + ) + except DatabaseError: + raise KeyValueMultiSaveError([]) + finally: + self._cache.update(pending_updates) + + @contract(kvs_key=DjangoKeyValueStore.Key) + def get(self, kvs_key): + """ + Return the django model object specified by `kvs_key` from + the cache. + + Arguments: + kvs_key (`DjangoKeyValueStore.Key`): The field value to delete + + Returns: A django orm object from the cache + """ + cache_key = self._cache_key_for_kvs_key(kvs_key) + if cache_key not in self._cache: + raise KeyError(kvs_key.field_name) + + return self._cache[cache_key][kvs_key.field_name] + + @contract(kvs_key=DjangoKeyValueStore.Key) + def delete(self, kvs_key): + """ + Delete the value specified by `kvs_key`. + + Arguments: + kvs_key (`DjangoKeyValueStore.Key`): The field value to delete + + Raises: KeyError if key isn't found in the cache + """ + cache_key = self._cache_key_for_kvs_key(kvs_key) + if cache_key not in self._cache: + raise KeyError(kvs_key.field_name) + + field_state = self._cache[cache_key] + + if kvs_key.field_name not in field_state: + raise KeyError(kvs_key.field_name) + + self._client.delete(self.user.username, cache_key, fields=[kvs_key.field_name]) + del field_state[kvs_key.field_name] + + @contract(kvs_key=DjangoKeyValueStore.Key, returns=bool) + def has(self, kvs_key): + """ + Return whether the specified `kvs_key` is set. + + Arguments: + kvs_key (`DjangoKeyValueStore.Key`): The field value to delete + + Returns: bool + """ + cache_key = self._cache_key_for_kvs_key(kvs_key) + + return ( + cache_key in self._cache and + kvs_key.field_name in self._cache[cache_key] + ) + + @contract(user_id=int, usage_key=UsageKey, score="number|None", max_score="number|None") + def set_score(self, user_id, usage_key, score, max_score): + """ + UNSUPPORTED METHOD + + Set the score and max_score for the specified user and xblock usage. + """ + student_module, created = StudentModule.objects.get_or_create( + student_id=user_id, + module_state_key=usage_key, + course_id=usage_key.course_key, + defaults={ + 'grade': score, + 'max_grade': max_score, + } + ) + if not created: + student_module.grade = score + student_module.max_grade = max_score + student_module.save() + + def __len__(self): + return len(self._cache) + + def _cache_key_for_kvs_key(self, key): + """ + Return the key used in this DjangoOrmFieldCache for the specified KeyValueStore key. + + Arguments: + key (:class:`~DjangoKeyValueStore.Key`): The key representing the cached field + """ + return key.block_scope_id + + +class UserStateSummaryCache(DjangoOrmFieldCache): + """ + Cache for Scope.user_state_summary xblock field data. + """ + def __init__(self, course_id): + super(UserStateSummaryCache, self).__init__() + self.course_id = course_id + + def _create_object(self, kvs_key, value): + """ + Create a new object to add to the cache (which should record + the specified field ``value`` for the field identified by + ``kvs_key``). + + Arguments: + kvs_key (:class:`DjangoKeyValueStore.Key`): Which field to create an entry for + value: The value to assign to the new field object + """ + return XModuleUserStateSummaryField( + field_name=kvs_key.field_name, + usage_id=kvs_key.block_scope_id, + value=value, + ) + + def _read_objects(self, fields, xblocks, aside_types): + """ + Return an iterator for all objects stored in the underlying datastore + for the ``fields`` on the ``xblocks`` and the ``aside_types`` associated + with them. + + Arguments: + fields (list of str): Field names to return values for + xblocks (list of :class:`~XBlock`): XBlocks to load fields for + aside_types (list of str): Asides to load field for (which annotate the supplied + xblocks). + """ + return XModuleUserStateSummaryField.objects.chunked_filter( + 'usage_id__in', + _all_usage_keys(xblocks, aside_types), + field_name__in=set(field.name for field in fields), + ) + + def _cache_key_for_field_object(self, field_object): + """ + Return the key used in this DjangoOrmFieldCache to store the specified field_object. + + Arguments: + field_object: A Django model instance that stores the data for fields in this cache + """ + return (field_object.usage_id.map_into_course(self.course_id), field_object.field_name) + + def _cache_key_for_kvs_key(self, key): + """ + Return the key used in this DjangoOrmFieldCache for the specified KeyValueStore key. + + Arguments: + key (:class:`~DjangoKeyValueStore.Key`): The key representing the cached field + """ + return (key.block_scope_id, key.field_name) + + +class PreferencesCache(DjangoOrmFieldCache): + """ + Cache for Scope.preferences xblock field data. + """ + def __init__(self, user): + super(PreferencesCache, self).__init__() + self.user = user + + def _create_object(self, kvs_key, value): + """ + Create a new object to add to the cache (which should record + the specified field ``value`` for the field identified by + ``kvs_key``). + + Arguments: + kvs_key (:class:`DjangoKeyValueStore.Key`): Which field to create an entry for + value: The value to assign to the new field object + """ + return XModuleStudentPrefsField( + field_name=kvs_key.field_name, + module_type=BlockTypeKeyV1(kvs_key.block_family, kvs_key.block_scope_id), + student_id=kvs_key.user_id, + value=value, + ) + + def _read_objects(self, fields, xblocks, aside_types): + """ + Return an iterator for all objects stored in the underlying datastore + for the ``fields`` on the ``xblocks`` and the ``aside_types`` associated + with them. + + Arguments: + fields (list of str): Field names to return values for + xblocks (list of :class:`~XBlock`): XBlocks to load fields for + aside_types (list of str): Asides to load field for (which annotate the supplied + xblocks). + """ + return XModuleStudentPrefsField.objects.chunked_filter( + 'module_type__in', + _all_block_types(xblocks, aside_types), + student=self.user.pk, + field_name__in=set(field.name for field in fields), + ) + + def _cache_key_for_field_object(self, field_object): + """ + Return the key used in this DjangoOrmFieldCache to store the specified field_object. + + Arguments: + field_object: A Django model instance that stores the data for fields in this cache + """ + return (field_object.module_type, field_object.field_name) + + def _cache_key_for_kvs_key(self, key): + """ + Return the key used in this DjangoOrmFieldCache for the specified KeyValueStore key. + + Arguments: + key (:class:`~DjangoKeyValueStore.Key`): The key representing the cached field + """ + return (BlockTypeKeyV1(key.block_family, key.block_scope_id), key.field_name) + + +class UserInfoCache(DjangoOrmFieldCache): + """ + Cache for Scope.user_info xblock field data + """ + def __init__(self, user): + super(UserInfoCache, self).__init__() + self.user = user + + def _create_object(self, kvs_key, value): + """ + Create a new object to add to the cache (which should record + the specified field ``value`` for the field identified by + ``kvs_key``). + + Arguments: + kvs_key (:class:`DjangoKeyValueStore.Key`): Which field to create an entry for + value: The value to assign to the new field object + """ + return XModuleStudentInfoField( + field_name=kvs_key.field_name, + student_id=kvs_key.user_id, + value=value, + ) + + def _read_objects(self, fields, xblocks, aside_types): + """ + Return an iterator for all objects stored in the underlying datastore + for the ``fields`` on the ``xblocks`` and the ``aside_types`` associated + with them. + + Arguments: + fields (list of str): Field names to return values for + xblocks (list of :class:`~XBlock`): XBlocks to load fields for + aside_types (list of str): Asides to load field for (which annotate the supplied + xblocks). + """ + return XModuleStudentInfoField.objects.filter( + student=self.user.pk, + field_name__in=set(field.name for field in fields), + ) + + def _cache_key_for_field_object(self, field_object): + """ + Return the key used in this DjangoOrmFieldCache to store the specified field_object. + + Arguments: + field_object: A Django model instance that stores the data for fields in this cache + """ + return field_object.field_name + + def _cache_key_for_kvs_key(self, key): + """ + Return the key used in this DjangoOrmFieldCache for the specified KeyValueStore key. + + Arguments: + key (:class:`~DjangoKeyValueStore.Key`): The key representing the cached field + """ + return key.field_name class FieldDataCache(object): """ A cache of django model objects needed to supply the data - for a module and its decendants + for a module and its descendants """ def __init__(self, descriptors, course_id, user, select_for_update=False, asides=None): - ''' + """ Find any courseware.models objects that are needed by any descriptor in descriptors. Attempts to minimize the number of queries to the database. Note: Only modules that have store_state = True or have shared @@ -58,12 +713,9 @@ class FieldDataCache(object): descriptors: A list of XModuleDescriptors. course_id: The id of the current course user: The user for which to cache data - select_for_update: True if rows should be locked until end of transaction + select_for_update: Ignored asides: The list of aside types to load, or None to prefetch no asides. - ''' - self.cache = {} - self.select_for_update = select_for_update - + """ if asides is None: self.asides = [] else: @@ -73,6 +725,21 @@ class FieldDataCache(object): self.course_id = course_id self.user = user + self.cache = { + Scope.user_state: UserStateCache( + self.user, + self.course_id, + ), + Scope.user_info: UserInfoCache( + self.user, + ), + Scope.preferences: PreferencesCache( + self.user, + ), + Scope.user_state_summary: UserStateSummaryCache( + self.course_id, + ), + } self.add_descriptors_to_cache(descriptors) def add_descriptors_to_cache(self, descriptors): @@ -81,18 +748,20 @@ class FieldDataCache(object): """ if self.user.is_authenticated(): for scope, fields in self._fields_to_cache(descriptors).items(): - for field_object in self._retrieve_fields(scope, fields, descriptors): - self.cache[self._cache_key_from_field_object(scope, field_object)] = field_object + if scope not in self.cache: + continue + + self.cache[scope].cache_fields(fields, descriptors, self.asides) def add_descriptor_descendents(self, descriptor, depth=None, descriptor_filter=lambda descriptor: True): """ - Add all descendents of `descriptor` to this FieldDataCache. + Add all descendants of `descriptor` to this FieldDataCache. Arguments: descriptor: An XModuleDescriptor - depth is the number of levels of descendent modules to load StudentModules for, in addition to - the supplied descriptor. If depth is None, load all descendent StudentModules - descriptor_filter is a function that accepts a descriptor and return wether the StudentModule + depth is the number of levels of descendant modules to load StudentModules for, in addition to + the supplied descriptor. If depth is None, load all descendant StudentModules + descriptor_filter is a function that accepts a descriptor and return whether the field data should be cached """ @@ -132,104 +801,16 @@ class FieldDataCache(object): course_id: the course in the context of which we want StudentModules. user: the django user for whom to load modules. descriptor: An XModuleDescriptor - depth is the number of levels of descendent modules to load StudentModules for, in addition to - the supplied descriptor. If depth is None, load all descendent StudentModules - descriptor_filter is a function that accepts a descriptor and return wether the StudentModule + depth is the number of levels of descendant modules to load StudentModules for, in addition to + the supplied descriptor. If depth is None, load all descendant StudentModules + descriptor_filter is a function that accepts a descriptor and return whether the field data should be cached - select_for_update: Flag indicating whether the rows should be locked until end of transaction + select_for_update: Ignored """ cache = FieldDataCache([], course_id, user, select_for_update, asides=asides) cache.add_descriptor_descendents(descriptor, depth, descriptor_filter) return cache - def _query(self, model_class, **kwargs): - """ - Queries model_class with **kwargs, optionally adding select_for_update if - self.select_for_update is set - """ - query = model_class.objects - if self.select_for_update: - query = query.select_for_update() - query = query.filter(**kwargs) - return query - - def _chunked_query(self, model_class, chunk_field, items, chunk_size=500, **kwargs): - """ - Queries model_class with `chunk_field` set to chunks of size `chunk_size`, - and all other parameters from `**kwargs` - - This works around a limitation in sqlite3 on the number of parameters - that can be put into a single query - """ - res = chain.from_iterable( - self._query(model_class, **dict([(chunk_field, chunk)] + kwargs.items())) - for chunk in chunks(items, chunk_size) - ) - return res - - def _all_usage_ids(self, descriptors): - """ - Return a set of all usage_ids for the descriptors that this FieldDataCache is caching - against, and well as all asides for those descriptors. - """ - usage_ids = set() - for descriptor in descriptors: - usage_ids.add(descriptor.scope_ids.usage_id) - - for aside_type in self.asides: - usage_ids.add(AsideUsageKeyV1(descriptor.scope_ids.usage_id, aside_type)) - - return usage_ids - - def _all_block_types(self, descriptors): - """ - Return a set of all block_types that are cached by this FieldDataCache. - """ - block_types = set() - for descriptor in descriptors: - block_types.add(BlockTypeKeyV1(descriptor.entry_point, descriptor.scope_ids.block_type)) - - for aside_type in self.asides: - block_types.add(BlockTypeKeyV1(XBlockAside.entry_point, aside_type)) - - return block_types - - def _retrieve_fields(self, scope, fields, descriptors): - """ - Queries the database for all of the fields in the specified scope - """ - if scope == Scope.user_state: - return self._chunked_query( - StudentModule, - 'module_state_key__in', - self._all_usage_ids(descriptors), - course_id=self.course_id, - student=self.user.pk, - ) - elif scope == Scope.user_state_summary: - return self._chunked_query( - XModuleUserStateSummaryField, - 'usage_id__in', - self._all_usage_ids(descriptors), - field_name__in=set(field.name for field in fields), - ) - elif scope == Scope.preferences: - return self._chunked_query( - XModuleStudentPrefsField, - 'module_type__in', - self._all_block_types(descriptors), - student=self.user.pk, - field_name__in=set(field.name for field in fields), - ) - elif scope == Scope.user_info: - return self._query( - XModuleStudentInfoField, - student=self.user.pk, - field_name__in=set(field.name for field in fields), - ) - else: - return [] - def _fields_to_cache(self, descriptors): """ Returns a map of scopes to fields in that scope that should be cached @@ -240,206 +821,136 @@ class FieldDataCache(object): scope_map[field.scope].add(field) return scope_map - def _cache_key_from_kvs_key(self, key): + @contract(key=DjangoKeyValueStore.Key) + def get(self, key): """ - Return the key used in the FieldDataCache for the specified KeyValueStore key + Load the field value specified by `key`. + + Arguments: + key (`DjangoKeyValueStore.Key`): The field value to load + + Returns: The found value + Raises: KeyError if key isn't found in the cache """ - if key.scope == Scope.user_state: - return (key.scope, key.block_scope_id) - elif key.scope == Scope.user_state_summary: - return (key.scope, key.block_scope_id, key.field_name) - elif key.scope == Scope.preferences: - return (key.scope, BlockTypeKeyV1(key.block_family, key.block_scope_id), key.field_name) - elif key.scope == Scope.user_info: - return (key.scope, key.field_name) - def _cache_key_from_field_object(self, scope, field_object): - """ - Return the key used in the FieldDataCache for the specified scope and - field - """ - if scope == Scope.user_state: - return (scope, field_object.module_state_key.map_into_course(self.course_id)) - elif scope == Scope.user_state_summary: - return (scope, field_object.usage_id.map_into_course(self.course_id), field_object.field_name) - elif scope == Scope.preferences: - return (scope, field_object.module_type, field_object.field_name) - elif scope == Scope.user_info: - return (scope, field_object.field_name) - - def find(self, key): - ''' - Look for a model data object using an DjangoKeyValueStore.Key object - - key: An `DjangoKeyValueStore.Key` object selecting the object to find - - returns the found object, or None if the object doesn't exist - ''' if key.scope.user == UserScope.ONE and not self.user.is_anonymous(): # If we're getting user data, we expect that the key matches the # user we were constructed for. assert key.user_id == self.user.id - return self.cache.get(self._cache_key_from_kvs_key(key)) - - def find_or_create(self, key): - ''' - Find a model data object in this cache, or create it if it doesn't - exist - ''' - field_object = self.find(key) - - if field_object is not None: - return field_object - - if key.scope == Scope.user_state: - field_object, __ = StudentModule.objects.get_or_create( - course_id=self.course_id, - student_id=key.user_id, - module_state_key=key.block_scope_id, - defaults={ - 'state': json.dumps({}), - 'module_type': key.block_scope_id.block_type, - }, - ) - elif key.scope == Scope.user_state_summary: - field_object, __ = XModuleUserStateSummaryField.objects.get_or_create( - field_name=key.field_name, - usage_id=key.block_scope_id - ) - elif key.scope == Scope.preferences: - field_object, __ = XModuleStudentPrefsField.objects.get_or_create( - field_name=key.field_name, - module_type=BlockTypeKeyV1(key.block_family, key.block_scope_id), - student_id=key.user_id, - ) - elif key.scope == Scope.user_info: - field_object, __ = XModuleStudentInfoField.objects.get_or_create( - field_name=key.field_name, - student_id=key.user_id, - ) - - cache_key = self._cache_key_from_kvs_key(key) - self.cache[cache_key] = field_object - return field_object - - -class DjangoKeyValueStore(KeyValueStore): - """ - This KeyValueStore will read and write data in the following scopes to django models - Scope.user_state_summary - Scope.user_state - Scope.preferences - Scope.user_info - - Access to any other scopes will raise an InvalidScopeError - - Data for Scope.user_state is stored as StudentModule objects via the django orm. - - Data for the other scopes is stored in individual objects that are named for the - scope involved and have the field name as a key - - If the key isn't found in the expected table during a read or a delete, then a KeyError will be raised - """ - - _allowed_scopes = ( - Scope.user_state_summary, - Scope.user_state, - Scope.preferences, - Scope.user_info, - ) - - def __init__(self, field_data_cache): - self._field_data_cache = field_data_cache - - def get(self, key): - if key.scope not in self._allowed_scopes: - raise InvalidScopeError(key) - - field_object = self._field_data_cache.find(key) - if field_object is None: + if key.scope not in self.cache: raise KeyError(key.field_name) - if key.scope == Scope.user_state: - return json.loads(field_object.state)[key.field_name] - else: - return json.loads(field_object.value) - - def set(self, key, value): - """ - Set a single value in the KeyValueStore - """ - self.set_many({key: value}) + return self.cache[key.scope].get(key) + @contract(kv_dict="dict(DjangoKeyValueStore_Key: *)") def set_many(self, kv_dict): """ - Provide a bulk save mechanism. - - `kv_dict`: A dictionary of dirty fields that maps - xblock.KvsFieldData._key : value + Set all of the fields specified by the keys of `kv_dict` to the values + in that dict. + Arguments: + kv_dict (dict): dict mapping from `DjangoKeyValueStore.Key`s to field values + Raises: DatabaseError if any fields fail to save """ + saved_fields = [] - # field_objects maps a field_object to a list of associated fields - field_objects = dict() - for field in kv_dict: - # Check field for validity - if field.scope not in self._allowed_scopes: - raise InvalidScopeError(field) + by_scope = defaultdict(dict) + for key, value in kv_dict.iteritems(): - # If the field is valid and isn't already in the dictionary, add it. - field_object = self._field_data_cache.find_or_create(field) - if field_object not in field_objects.keys(): - field_objects[field_object] = [] - # Update the list of associated fields - field_objects[field_object].append(field) + if key.scope.user == UserScope.ONE and not self.user.is_anonymous(): + # If we're getting user data, we expect that the key matches the + # user we were constructed for. + assert key.user_id == self.user.id - # Special case when scope is for the user state, because this scope saves fields in a single row - if field.scope == Scope.user_state: - state = json.loads(field_object.state) - state[field.field_name] = kv_dict[field] - field_object.state = json.dumps(state) - else: - # The remaining scopes save fields on different rows, so - # we don't have to worry about conflicts - field_object.value = json.dumps(kv_dict[field]) + if key.scope not in self.cache: + continue - for field_object in field_objects: + by_scope[key.scope][key] = value + + for scope, set_many_data in by_scope.iteritems(): try: - # Save the field object that we made above - field_object.save() - # If save is successful on this scope, add the saved fields to + self.cache[scope].set_many(set_many_data) + # If save is successful on these fields, add it to # the list of successful saves - saved_fields.extend([field.field_name for field in field_objects[field_object]]) - except DatabaseError: - log.exception('Error saving fields %r', field_objects[field_object]) - raise KeyValueMultiSaveError(saved_fields) + saved_fields.extend(key.field_name for key in set_many_data) + except KeyValueMultiSaveError as exc: + log.exception('Error saving fields %r', [key.field_name for key in set_many_data]) + raise KeyValueMultiSaveError(saved_fields + exc.saved_field_names) + @contract(key=DjangoKeyValueStore.Key) def delete(self, key): - if key.scope not in self._allowed_scopes: - raise InvalidScopeError(key) + """ + Delete the value specified by `key`. - field_object = self._field_data_cache.find(key) - if field_object is None: + Arguments: + key (`DjangoKeyValueStore.Key`): The field value to delete + + Raises: KeyError if key isn't found in the cache + """ + + if key.scope.user == UserScope.ONE and not self.user.is_anonymous(): + # If we're getting user data, we expect that the key matches the + # user we were constructed for. + assert key.user_id == self.user.id + + if key.scope not in self.cache: raise KeyError(key.field_name) - if key.scope == Scope.user_state: - state = json.loads(field_object.state) - del state[key.field_name] - field_object.state = json.dumps(state) - field_object.save() - else: - field_object.delete() + self.cache[key.scope].delete(key) + @contract(key=DjangoKeyValueStore.Key, returns=bool) def has(self, key): - if key.scope not in self._allowed_scopes: - raise InvalidScopeError(key) + """ + Return whether the specified `key` is set. - field_object = self._field_data_cache.find(key) - if field_object is None: + Arguments: + key (`DjangoKeyValueStore.Key`): The field value to delete + + Returns: bool + """ + + if key.scope.user == UserScope.ONE and not self.user.is_anonymous(): + # If we're getting user data, we expect that the key matches the + # user we were constructed for. + assert key.user_id == self.user.id + + if key.scope not in self.cache: return False - if key.scope == Scope.user_state: - return key.field_name in json.loads(field_object.state) - else: - return True + return self.cache[key.scope].has(key) + + @contract(user_id=int, usage_key=UsageKey, score="number|None", max_score="number|None") + def set_score(self, user_id, usage_key, score, max_score): + """ + UNSUPPORTED METHOD + + Set the score and max_score for the specified user and xblock usage. + """ + assert not self.user.is_anonymous() + assert user_id == self.user.id + assert usage_key.course_key == self.course_id + self.cache[Scope.user_state].set_score(user_id, usage_key, score, max_score) + + @contract(key=DjangoKeyValueStore.Key, returns="datetime|None") + def last_modified(self, key): + """ + Return when the supplied field was changed. + + Arguments: + key (`DjangoKeyValueStore.Key`): The field value to delete + + Returns: datetime if there was a modified date, or None otherwise + """ + if key.scope.user == UserScope.ONE and not self.user.is_anonymous(): + # If we're getting user data, we expect that the key matches the + # user we were constructed for. + assert key.user_id == self.user.id + + if key.scope not in self.cache: + return None + + return self.cache[key.scope].last_modified(key) + + def __len__(self): + return sum(len(cache) for cache in self.cache.values()) diff --git a/lms/djangoapps/courseware/models.py b/lms/djangoapps/courseware/models.py index 030eecbfdc..3c2cd72026 100644 --- a/lms/djangoapps/courseware/models.py +++ b/lms/djangoapps/courseware/models.py @@ -13,6 +13,7 @@ ASSUMPTIONS: modules have unique IDs, even across different module_types """ import logging +import itertools from django.contrib.auth.models import User from django.conf import settings @@ -29,10 +30,49 @@ from xmodule_django.models import CourseKeyField, LocationKeyField, BlockTypeKey log = logging.getLogger("edx.courseware") +def chunks(items, chunk_size): + """ + Yields the values from items in chunks of size chunk_size + """ + items = list(items) + return (items[i:i + chunk_size] for i in xrange(0, len(items), chunk_size)) + + +class ChunkingManager(models.Manager): + """ + :class:`~Manager` that adds an additional method :meth:`chunked_filter` to provide + the ability to make select queries with specific chunk sizes. + """ + def chunked_filter(self, chunk_field, items, **kwargs): + """ + Queries model_class with `chunk_field` set to chunks of size `chunk_size`, + and all other parameters from `**kwargs`. + + This works around a limitation in sqlite3 on the number of parameters + that can be put into a single query. + + Arguments: + chunk_field (str): The name of the field to chunk the query on. + items: The values for of chunk_field to select. This is chunked into ``chunk_size`` + chunks, and passed as the value for the ``chunk_field`` keyword argument to + :meth:`~Manager.filter`. This implies that ``chunk_field`` should be an + ``__in`` key. + chunk_size (int): The size of chunks to pass. Defaults to 500. + """ + chunk_size = kwargs.pop('chunk_size', 500) + res = itertools.chain.from_iterable( + self.filter(**dict([(chunk_field, chunk)] + kwargs.items())) + for chunk in chunks(items, chunk_size) + ) + return res + + class StudentModule(models.Model): """ Keeps student state for a particular module in a particular course. """ + objects = ChunkingManager() + MODEL_TAGS = ['course_id', 'module_type'] # For a homework problem, contains a JSON @@ -142,6 +182,8 @@ class XBlockFieldBase(models.Model): """ Base class for all XBlock field storage. """ + objects = ChunkingManager() + class Meta(object): # pylint: disable=missing-docstring abstract = True diff --git a/lms/djangoapps/courseware/module_render.py b/lms/djangoapps/courseware/module_render.py index ada35ac1f7..03e256c742 100644 --- a/lms/djangoapps/courseware/module_render.py +++ b/lms/djangoapps/courseware/module_render.py @@ -52,7 +52,7 @@ from xblock.exceptions import NoSuchHandlerError, NoSuchViewError from xblock.django.request import django_to_webob_request, webob_to_django_response from xmodule.error_module import ErrorDescriptor, NonStaffErrorDescriptor from xmodule.exceptions import NotFoundError, ProcessingError -from opaque_keys.edx.keys import UsageKey +from opaque_keys.edx.keys import UsageKey, CourseKey from opaque_keys.edx.locations import SlashSeparatedCourseKey from xmodule.contentstore.django import contentstore from xmodule.modulestore.django import modulestore, ModuleI18nService @@ -417,23 +417,18 @@ def get_module_system_for_user(user, field_data_cache, """ user_id = event.get('user_id', user.id) - # Construct the key for the module - key = KeyValueStore.Key( - scope=Scope.user_state, - user_id=user_id, - block_scope_id=descriptor.location, - field_name='grade' + grade = event.get('value') + max_grade = event.get('max_value') + + field_data_cache.set_score( + user_id, + descriptor.location, + grade, + max_grade, ) - student_module = field_data_cache.find_or_create(key) - # Update the grades - student_module.grade = event.get('value') - student_module.max_grade = event.get('max_value') - # Save all changes to the underlying KeyValueStore - student_module.save() - # Bin score into range and increment stats - score_bucket = get_score_bucket(student_module.grade, student_module.max_grade) + score_bucket = get_score_bucket(grade, max_grade) tags = [ u"org:{}".format(course_id.org), @@ -733,23 +728,23 @@ def get_module_for_descriptor_internal(user, descriptor, field_data_cache, cours return descriptor -def find_target_student_module(request, user_id, course_id, mod_id): +def load_single_xblock(request, user_id, course_id, usage_key_string): """ - Retrieve target StudentModule + Load a single XBlock identified by usage_key_string. """ - course_id = SlashSeparatedCourseKey.from_deprecated_string(course_id) - usage_key = course_id.make_usage_key_from_deprecated_string(mod_id) + usage_key = UsageKey.from_string(usage_key_string) + course_key = CourseKey.from_string(course_id) + usage_key = usage_key.map_into_course(course_key) user = User.objects.get(id=user_id) field_data_cache = FieldDataCache.cache_for_descriptor_descendents( - course_id, + course_key, user, modulestore().get_item(usage_key), depth=0, - select_for_update=True ) instance = get_module(user, request, usage_key, field_data_cache, grade_bucket_type='xqueue') if instance is None: - msg = "No module {0} for user {1}--access denied?".format(mod_id, user) + msg = "No module {0} for user {1}--access denied?".format(usage_key_string, user) log.debug(msg) raise Http404 return instance @@ -773,7 +768,7 @@ def xqueue_callback(request, course_id, userid, mod_id, dispatch): if not isinstance(header, dict) or 'lms_key' not in header: raise Http404 - instance = find_target_student_module(request, userid, course_id, mod_id) + instance = load_single_xblock(request, userid, course_id, mod_id) # Transfer 'queuekey' from xqueue response header to the data. # This is required to use the interface defined by 'handle_ajax' diff --git a/lms/djangoapps/courseware/tests/test_model_data.py b/lms/djangoapps/courseware/tests/test_model_data.py index ca62da4e80..0f87c49de3 100644 --- a/lms/djangoapps/courseware/tests/test_model_data.py +++ b/lms/djangoapps/courseware/tests/test_model_data.py @@ -134,7 +134,10 @@ class TestStudentModuleStorage(OtherUserFailureTestMixin, TestCase): def test_set_existing_field(self): "Test that setting an existing user_state field changes the value" # We are updating a problem, so we write to courseware_studentmodulehistory - # as well as courseware_studentmodule + # as well as courseware_studentmodule. We also need to read the database + # to discover if something other than the DjangoXBlockUserStateClient + # has written to the StudentModule (such as UserStateCache setting the score + # on the StudentModule). with self.assertNumQueries(3): self.kvs.set(user_state_key('a_field'), 'new_value') self.assertEquals(1, StudentModule.objects.all().count()) @@ -143,7 +146,10 @@ class TestStudentModuleStorage(OtherUserFailureTestMixin, TestCase): def test_set_missing_field(self): "Test that setting a new user_state field changes the value" # We are updating a problem, so we write to courseware_studentmodulehistory - # as well as courseware_studentmodule + # as well as courseware_studentmodule. We also need to read the database + # to discover if something other than the DjangoXBlockUserStateClient + # has written to the StudentModule (such as UserStateCache setting the score + # on the StudentModule). with self.assertNumQueries(3): self.kvs.set(user_state_key('not_a_field'), 'new_value') self.assertEquals(1, StudentModule.objects.all().count()) @@ -152,7 +158,10 @@ class TestStudentModuleStorage(OtherUserFailureTestMixin, TestCase): def test_delete_existing_field(self): "Test that deleting an existing field removes it from the StudentModule" # We are updating a problem, so we write to courseware_studentmodulehistory - # as well as courseware_studentmodule + # as well as courseware_studentmodule. We also need to read the database + # to discover if something other than the DjangoXBlockUserStateClient + # has written to the StudentModule (such as UserStateCache setting the score + # on the StudentModule). with self.assertNumQueries(3): self.kvs.delete(user_state_key('a_field')) self.assertEquals(1, StudentModule.objects.all().count()) @@ -190,6 +199,9 @@ class TestStudentModuleStorage(OtherUserFailureTestMixin, TestCase): # Scope.user_state is stored in a single row in the database, so we only # need to send a single update to that table. # We also are updating a problem, so we write to courseware student module history + # We also need to read the database to discover if something other than the + # DjangoXBlockUserStateClient has written to the StudentModule (such as + # UserStateCache setting the score on the StudentModule). with self.assertNumQueries(3): self.kvs.set_many(kv_dict) @@ -207,7 +219,7 @@ class TestStudentModuleStorage(OtherUserFailureTestMixin, TestCase): with patch('django.db.models.Model.save', side_effect=DatabaseError): with self.assertRaises(KeyValueMultiSaveError) as exception_context: self.kvs.set_many(kv_dict) - self.assertEquals(len(exception_context.exception.saved_field_names), 0) + self.assertEquals(exception_context.exception.saved_field_names, []) @attr('shard_1') @@ -230,15 +242,18 @@ class TestMissingStudentModule(TestCase): def test_set_field_in_missing_student_module(self): "Test that setting a field in a missing StudentModule creates the student module" - self.assertEquals(0, len(self.field_data_cache.cache)) + self.assertEquals(0, len(self.field_data_cache)) self.assertEquals(0, StudentModule.objects.all().count()) # We are updating a problem, so we write to courseware_studentmodulehistory - # as well as courseware_studentmodule - with self.assertNumQueries(6): + # as well as courseware_studentmodule. We also need to read the database + # to discover if something other than the DjangoXBlockUserStateClient + # has written to the StudentModule (such as UserStateCache setting the score + # on the StudentModule). + with self.assertNumQueries(3): self.kvs.set(user_state_key('a_field'), 'a_value') - self.assertEquals(1, len(self.field_data_cache.cache)) + self.assertEquals(1, sum(len(cache) for cache in self.field_data_cache.cache.values())) self.assertEquals(1, StudentModule.objects.all().count()) student_module = StudentModule.objects.all()[0] @@ -289,7 +304,7 @@ class StorageTestBase(object): self.kvs = DjangoKeyValueStore(self.field_data_cache) def test_set_and_get_existing_field(self): - with self.assertNumQueries(2): + with self.assertNumQueries(1): self.kvs.set(self.key_factory('existing_field'), 'test_value') with self.assertNumQueries(0): self.assertEquals('test_value', self.kvs.get(self.key_factory('existing_field'))) @@ -306,14 +321,14 @@ class StorageTestBase(object): def test_set_existing_field(self): "Test that setting an existing field changes the value" - with self.assertNumQueries(2): + with self.assertNumQueries(1): self.kvs.set(self.key_factory('existing_field'), 'new_value') self.assertEquals(1, self.storage_class.objects.all().count()) self.assertEquals('new_value', json.loads(self.storage_class.objects.all()[0].value)) def test_set_missing_field(self): "Test that setting a new field changes the value" - with self.assertNumQueries(4): + with self.assertNumQueries(1): self.kvs.set(self.key_factory('missing_field'), 'new_value') self.assertEquals(2, self.storage_class.objects.all().count()) self.assertEquals('old_value', json.loads(self.storage_class.objects.get(field_name='existing_field').value)) @@ -355,7 +370,7 @@ class StorageTestBase(object): # Each field is a separate row in the database, hence # a separate query - with self.assertNumQueries(len(kv_dict) * 3): + with self.assertNumQueries(len(kv_dict)): self.kvs.set_many(kv_dict) for key in kv_dict: self.assertEquals(self.kvs.get(key), kv_dict[key]) @@ -363,8 +378,8 @@ class StorageTestBase(object): def test_set_many_failure(self): """Test that setting many regular fields with a DB error """ kv_dict = self.construct_kv_dict() - with self.assertNumQueries(6): - for key in kv_dict: + for key in kv_dict: + with self.assertNumQueries(1): self.kvs.set(key, 'test value') with patch('django.db.models.Model.save', side_effect=[None, DatabaseError]): @@ -372,8 +387,7 @@ class StorageTestBase(object): self.kvs.set_many(kv_dict) exception = exception_context.exception - self.assertEquals(len(exception.saved_field_names), 1) - self.assertEquals(exception.saved_field_names[0], 'existing_field') + self.assertEquals(exception.saved_field_names, ['existing_field', 'other_existing_field']) class TestUserStateSummaryStorage(StorageTestBase, TestCase): diff --git a/lms/djangoapps/courseware/tests/test_module_render.py b/lms/djangoapps/courseware/tests/test_module_render.py index 68117b06f4..99008a823a 100644 --- a/lms/djangoapps/courseware/tests/test_module_render.py +++ b/lms/djangoapps/courseware/tests/test_module_render.py @@ -48,7 +48,7 @@ from xmodule.modulestore import ModuleStoreEnum from xmodule.modulestore.django import modulestore from xmodule.modulestore.tests.django_utils import ModuleStoreTestCase from xmodule.modulestore.tests.factories import ItemFactory, CourseFactory, check_mongo_calls -from xmodule.x_module import XModuleDescriptor, XModule, STUDENT_VIEW, CombinedSystem +from xmodule.x_module import XModuleDescriptor, XModule, STUDENT_VIEW, CombinedSystem, DescriptorSystem TEST_DATA_DIR = settings.COMMON_TEST_DATA_ROOT @@ -78,6 +78,27 @@ class EmptyXModuleDescriptor(XModuleDescriptor): # pylint: disable=abstract-met module_class = EmptyXModule +class GradedStatelessXBlock(XBlock): + """ + This XBlock exists to test grade storage for blocks that don't store + student state in a scoped field. + """ + + @XBlock.json_handler + def set_score(self, json_data, suffix): # pylint: disable=unused-argument + """ + Set the score for this testing XBlock. + """ + self.runtime.publish( + self, + 'grade', + { + 'value': json_data['grade'], + 'max_value': 1 + } + ) + + @attr('shard_1') @ddt.ddt class ModuleRenderTestCase(ModuleStoreTestCase, LoginEnrollmentTestCase): @@ -160,8 +181,7 @@ class ModuleRenderTestCase(ModuleStoreTestCase, LoginEnrollmentTestCase): } # Patch getmodule to return our mock module - with patch('courseware.module_render.find_target_student_module') as get_fake_module: - get_fake_module.return_value = self.mock_module + with patch('courseware.module_render.load_single_xblock', return_value=self.mock_module): # call xqueue_callback with our mocked information request = self.request_factory.post(self.callback_url, data) render.xqueue_callback(request, self.course_key, self.mock_user.id, self.mock_module.id, self.dispatch) @@ -176,8 +196,7 @@ class ModuleRenderTestCase(ModuleStoreTestCase, LoginEnrollmentTestCase): 'xqueue_body': 'hello world', } - with patch('courseware.module_render.find_target_student_module') as get_fake_module: - get_fake_module.return_value = self.mock_module + with patch('courseware.module_render.load_single_xblock', return_value=self.mock_module): # Test with missing xqueue data with self.assertRaises(Http404): request = self.request_factory.post(self.callback_url, {}) @@ -337,8 +356,7 @@ class TestHandleXBlockCallback(ModuleStoreTestCase, LoginEnrollmentTestCase): self.course_key = self.create_toy_course() self.location = self.course_key.make_usage_key('chapter', 'Overview') self.toy_course = modulestore().get_course(self.course_key) - self.mock_user = UserFactory() - self.mock_user.id = 1 + self.mock_user = UserFactory.create() self.request_factory = RequestFactory() # Construct a mock module for the modulestore to return @@ -478,6 +496,33 @@ class TestHandleXBlockCallback(ModuleStoreTestCase, LoginEnrollmentTestCase): 'bad_dispatch', ) + @XBlock.register_temp_plugin(GradedStatelessXBlock, identifier='stateless_scorer') + def test_score_without_student_state(self): + course = CourseFactory.create() + block = ItemFactory.create(category='stateless_scorer', parent=course) + + request = self.request_factory.post( + 'dummy_url', + data=json.dumps({"grade": 0.75}), + content_type='application/json' + ) + request.user = self.mock_user + + response = render.handle_xblock_callback( + request, + unicode(course.id), + quote_slashes(unicode(block.scope_ids.usage_id)), + 'set_score', + '', + ) + self.assertEquals(response.status_code, 200) + student_module = StudentModule.objects.get( + student=self.mock_user, + module_state_key=block.scope_ids.usage_id, + ) + self.assertEquals(student_module.grade, 0.75) + self.assertEquals(student_module.max_grade, 1) + @patch.dict('django.conf.settings.FEATURES', {'ENABLE_XBLOCK_VIEW_ENDPOINT': True}) def test_xblock_view_handler(self): args = [ @@ -1063,7 +1108,7 @@ class TestAnonymousStudentId(ModuleStoreTestCase, LoginEnrollmentTestCase): location=location, static_asset_path=None, _runtime=Mock( - spec=Runtime, + spec=DescriptorSystem, resources_fs=None, mixologist=Mock(_mixins=(), name='mixologist'), name='runtime', diff --git a/lms/djangoapps/courseware/user_state_client.py b/lms/djangoapps/courseware/user_state_client.py new file mode 100644 index 0000000000..31137c9975 --- /dev/null +++ b/lms/djangoapps/courseware/user_state_client.py @@ -0,0 +1,331 @@ +""" +An implementation of :class:`XBlockUserStateClient`, which stores XBlock Scope.user_state +data in a Django ORM model. +""" + +import itertools +from operator import attrgetter + +try: + import simplejson as json +except ImportError: + import json + +from xblock.fields import Scope, ScopeBase +from xblock_user_state.interface import XBlockUserStateClient +from courseware.models import StudentModule +from contracts import contract, new_contract +from opaque_keys.edx.keys import UsageKey + +new_contract('UsageKey', UsageKey) + + +class DjangoXBlockUserStateClient(XBlockUserStateClient): + """ + An interface that uses the Django ORM StudentModule as a backend. + """ + + class ServiceUnavailable(XBlockUserStateClient.ServiceUnavailable): + """ + This error is raised if the service backing this client is currently unavailable. + """ + pass + + class PermissionDenied(XBlockUserStateClient.PermissionDenied): + """ + This error is raised if the caller is not allowed to access the requested data. + """ + pass + + class DoesNotExist(XBlockUserStateClient.DoesNotExist): + """ + This error is raised if the caller has requested data that does not exist. + """ + pass + + def __init__(self, user): + self.user = user + + @contract( + username="basestring", + block_key=UsageKey, + scope=ScopeBase, + fields="seq(basestring)|set(basestring)|None" + ) + def get(self, username, block_key, scope=Scope.user_state, fields=None): + """ + Retrieve the stored XBlock state for a single xblock usage. + + Arguments: + username: The name of the user whose state should be retrieved + block_key (UsageKey): The UsageKey identifying which xblock state to load. + scope (Scope): The scope to load data from + fields: A list of field values to retrieve. If None, retrieve all stored fields. + + Returns: + dict: A dictionary mapping field names to values + + Raises: + DoesNotExist if no entry is found. + """ + assert self.user.username == username + try: + _usage_key, state = next(self.get_many(username, [block_key], scope, fields=fields)) + except StopIteration: + raise self.DoesNotExist() + + return state + + @contract(username="basestring", block_key=UsageKey, state="dict(basestring: *)", scope=ScopeBase) + def set(self, username, block_key, state, scope=Scope.user_state): + """ + Set fields for a particular XBlock. + + Arguments: + username: The name of the user whose state should be retrieved + block_key (UsageKey): The UsageKey identifying which xblock state to update. + state (dict): A dictionary mapping field names to values + scope (Scope): The scope to load data from + """ + assert self.user.username == username + self.set_many(username, {block_key: state}, scope) + + @contract( + username="basestring", + block_key=UsageKey, + scope=ScopeBase, + fields="seq(basestring)|set(basestring)|None" + ) + def delete(self, username, block_key, scope=Scope.user_state, fields=None): + """ + Delete the stored XBlock state for a single xblock usage. + + Arguments: + username: The name of the user whose state should be deleted + block_key (UsageKey): The UsageKey identifying which xblock state to delete. + scope (Scope): The scope to delete data from + fields: A list of fields to delete. If None, delete all stored fields. + """ + assert self.user.username == username + return self.delete_many(username, [block_key], scope, fields=fields) + + @contract( + username="basestring", + block_key=UsageKey, + scope=ScopeBase, + fields="seq(basestring)|set(basestring)|None" + ) + def get_mod_date(self, username, block_key, scope=Scope.user_state, fields=None): + """ + Get the last modification date for fields from the specified blocks. + + Arguments: + username: The name of the user whose state should be deleted + block_key (UsageKey): The UsageKey identifying which xblock modification dates to retrieve. + scope (Scope): The scope to retrieve from. + fields: A list of fields to query. If None, delete all stored fields. + Specific implementations are free to return the same modification date + for all fields, if they don't store changes individually per field. + Implementations may omit fields for which data has not been stored. + + Returns: list a dict of {field_name: modified_date} for each selected field. + """ + results = self.get_mod_date_many(username, [block_key], scope, fields=fields) + return { + field: date for (_, field, date) in results + } + + @contract(username="basestring", block_keys="seq(UsageKey)|set(UsageKey)") + def _get_student_modules(self, username, block_keys): + """ + Retrieve the :class:`~StudentModule`s for the supplied ``username`` and ``block_keys``. + + Arguments: + username (str): The name of the user to load `StudentModule`s for. + block_keys (list of :class:`~UsageKey`): The set of XBlocks to load data for. + """ + course_key_func = attrgetter('course_key') + by_course = itertools.groupby( + sorted(block_keys, key=course_key_func), + course_key_func, + ) + + for course_key, usage_keys in by_course: + query = StudentModule.objects.chunked_filter( + 'module_state_key__in', + usage_keys, + student__username=username, + course_id=course_key, + ) + + for student_module in query: + usage_key = student_module.module_state_key.map_into_course(student_module.course_id) + yield (student_module, usage_key) + + @contract( + username="basestring", + block_keys="seq(UsageKey)|set(UsageKey)", + scope=ScopeBase, + fields="seq(basestring)|set(basestring)|None" + ) + def get_many(self, username, block_keys, scope=Scope.user_state, fields=None): + """ + Retrieve the stored XBlock state for a single xblock usage. + + Arguments: + username: The name of the user whose state should be retrieved + block_keys ([UsageKey]): A list of UsageKeys identifying which xblock states to load. + scope (Scope): The scope to load data from + fields: A list of field values to retrieve. If None, retrieve all stored fields. + + Yields: + (UsageKey, field_state) tuples for each specified UsageKey in block_keys. + field_state is a dict mapping field names to values. + """ + assert self.user.username == username + if scope != Scope.user_state: + raise ValueError("Only Scope.user_state is supported, not {}".format(scope)) + + modules = self._get_student_modules(username, block_keys) + for module, usage_key in modules: + if module.state is None: + state = {} + else: + state = json.loads(module.state) + yield (usage_key, state) + + @contract(username="basestring", block_keys_to_state="dict(UsageKey: dict(basestring: *))", scope=ScopeBase) + def set_many(self, username, block_keys_to_state, scope=Scope.user_state): + """ + Set fields for a particular XBlock. + + Arguments: + username: The name of the user whose state should be retrieved + block_keys_to_state (dict): A dict mapping UsageKeys to state dicts. + Each state dict maps field names to values. These state dicts + are overlaid over the stored state. To delete fields, use + :meth:`delete` or :meth:`delete_many`. + scope (Scope): The scope to load data from + """ + assert self.user.username == username + if scope != Scope.user_state: + raise ValueError("Only Scope.user_state is supported") + + # We do a find_or_create for every block (rather than re-using field objects + # that were queried in get_many) so that if the score has + # been changed by some other piece of the code, we don't overwrite + # that score. + for usage_key, state in block_keys_to_state.items(): + student_module, created = StudentModule.objects.get_or_create( + student=self.user, + course_id=usage_key.course_key, + module_state_key=usage_key, + defaults={ + 'state': json.dumps(state), + 'module_type': usage_key.block_type, + }, + ) + + if not created: + if student_module.state is None: + current_state = {} + else: + current_state = json.loads(student_module.state) + current_state.update(state) + student_module.state = json.dumps(current_state) + # We just read this object, so we know that we can do an update + student_module.save(force_update=True) + + @contract( + username="basestring", + block_keys="seq(UsageKey)|set(UsageKey)", + scope=ScopeBase, + fields="seq(basestring)|set(basestring)|None" + ) + def delete_many(self, username, block_keys, scope=Scope.user_state, fields=None): + """ + Delete the stored XBlock state for a many xblock usages. + + Arguments: + username: The name of the user whose state should be deleted + block_key (UsageKey): The UsageKey identifying which xblock state to delete. + scope (Scope): The scope to delete data from + fields: A list of fields to delete. If None, delete all stored fields. + """ + assert self.user.username == username + if scope != Scope.user_state: + raise ValueError("Only Scope.user_state is supported") + + student_modules = self._get_student_modules(username, block_keys) + for student_module, _ in student_modules: + if fields is None: + student_module.state = "{}" + else: + current_state = json.loads(student_module.state) + for field in fields: + if field in current_state: + del current_state[field] + + student_module.state = json.dumps(current_state) + # We just read this object, so we know that we can do an update + student_module.save(force_update=True) + + @contract( + username="basestring", + block_keys="seq(UsageKey)|set(UsageKey)", + scope=ScopeBase, + fields="seq(basestring)|set(basestring)|None" + ) + def get_mod_date_many(self, username, block_keys, scope=Scope.user_state, fields=None): + """ + Get the last modification date for fields from the specified blocks. + + Arguments: + username: The name of the user whose state should be deleted + block_key (UsageKey): The UsageKey identifying which xblock modification dates to retrieve. + scope (Scope): The scope to retrieve from. + fields: A list of fields to query. If None, delete all stored fields. + Specific implementations are free to return the same modification date + for all fields, if they don't store changes individually per field. + Implementations may omit fields for which data has not been stored. + + Yields: tuples of (block, field_name, modified_date) for each selected field. + """ + assert self.user.username == username + if scope != Scope.user_state: + raise ValueError("Only Scope.user_state is supported") + + student_modules = self._get_student_modules(username, block_keys) + for student_module, usage_key in student_modules: + if student_module.state is None: + continue + + for field in json.loads(student_module.state): + yield (usage_key, field, student_module.modified) + + def get_history(self, username, block_key, scope=Scope.user_state): + """We don't guarantee that history for many blocks will be fast.""" + assert self.user.username == username + if scope != Scope.user_state: + raise ValueError("Only Scope.user_state is supported") + raise NotImplementedError() + + def iter_all_for_block(self, block_key, scope=Scope.user_state, batch_size=None): + """ + You get no ordering guarantees. Fetching will happen in batch_size + increments. If you're using this method, you should be running in an + async task. + """ + if scope != Scope.user_state: + raise ValueError("Only Scope.user_state is supported") + raise NotImplementedError() + + def iter_all_for_course(self, course_key, block_type=None, scope=Scope.user_state, batch_size=None): + """ + You get no ordering guarantees. Fetching will happen in batch_size + increments. If you're using this method, you should be running in an + async task. + """ + if scope != Scope.user_state: + raise ValueError("Only Scope.user_state is supported") + raise NotImplementedError() diff --git a/lms/djangoapps/lms_xblock/test/test_runtime.py b/lms/djangoapps/lms_xblock/test/test_runtime.py index a8fe5a9df2..6b3bb748ec 100644 --- a/lms/djangoapps/lms_xblock/test/test_runtime.py +++ b/lms/djangoapps/lms_xblock/test/test_runtime.py @@ -11,6 +11,7 @@ from urlparse import urlparse from opaque_keys.edx.locations import SlashSeparatedCourseKey from lms.djangoapps.lms_xblock.runtime import quote_slashes, unquote_slashes, LmsModuleSystem from xblock.fields import ScopeIds +from xmodule.x_module import DescriptorSystem TEST_STRINGS = [ '', @@ -48,12 +49,12 @@ class TestHandlerUrl(TestCase): self.course_key = SlashSeparatedCourseKey("org", "course", "run") self.runtime = LmsModuleSystem( static_url='/static', - track_function=Mock(), - get_module=Mock(), - render_template=Mock(), + track_function=Mock(name='track_function'), + get_module=Mock(name='get_module'), + render_template=Mock(name='render_template'), replace_urls=str, course_id=self.course_key, - descriptor_runtime=Mock(), + descriptor_runtime=Mock(spec=DescriptorSystem, name='descriptor_runtime'), ) def test_trailing_characters(self): @@ -120,13 +121,13 @@ class TestUserServiceAPI(TestCase): self.runtime = LmsModuleSystem( static_url='/static', - track_function=Mock(), - get_module=Mock(), - render_template=Mock(), + track_function=Mock(name="track_function"), + get_module=Mock(name="get_module"), + render_template=Mock(name="render_template"), replace_urls=str, course_id=self.course_id, get_real_user=mock_get_real_user, - descriptor_runtime=Mock(), + descriptor_runtime=Mock(spec=DescriptorSystem, name="descriptor_runtime"), ) self.scope = 'course' self.key = 'key1' diff --git a/lms/djangoapps/mobile_api/users/views.py b/lms/djangoapps/mobile_api/users/views.py index fa1e6c68ee..446d7a934d 100644 --- a/lms/djangoapps/mobile_api/users/views.py +++ b/lms/djangoapps/mobile_api/users/views.py @@ -147,14 +147,12 @@ class UserCourseStatus(views.APIView): scope=Scope.user_state, user_id=request.user.id, block_scope_id=course.location, - field_name=None + field_name='position' ) - student_module = field_data_cache.find(key) - if student_module: - original_store_date = student_module.modified - if modification_date < original_store_date: - # old modification date so skip update - return self._get_course_info(request, course) + original_store_date = field_data_cache.last_modified(key) + if original_store_date is not None and modification_date < original_store_date: + # old modification date so skip update + return self._get_course_info(request, course) save_positions_recursively_up(request.user, request, field_data_cache, module) return self._get_course_info(request, course) diff --git a/lms/djangoapps/open_ended_grading/tests.py b/lms/djangoapps/open_ended_grading/tests.py index 4614e8bf47..4f7114242f 100644 --- a/lms/djangoapps/open_ended_grading/tests.py +++ b/lms/djangoapps/open_ended_grading/tests.py @@ -24,6 +24,7 @@ from lms.djangoapps.lms_xblock.runtime import LmsModuleSystem from student.roles import CourseStaffRole from student.models import unique_id_for_user from xmodule import peer_grading_module +from xmodule.x_module import DescriptorSystem from xmodule.error_module import ErrorDescriptor from xmodule.modulestore.django import modulestore from xmodule.modulestore.tests.django_utils import ModuleStoreTestCase @@ -288,7 +289,7 @@ class TestPeerGradingService(ModuleStoreTestCase, LoginEnrollmentTestCase): open_ended_grading_interface=test_util_open_ended.OPEN_ENDED_GRADING_INTERFACE, mixins=settings.XBLOCK_MIXINS, error_descriptor_class=ErrorDescriptor, - descriptor_runtime=None, + descriptor_runtime=Mock(spec=DescriptorSystem, name="descriptor_runtime"), ) self.descriptor = peer_grading_module.PeerGradingDescriptor(self.system, field_data, ScopeIds(None, None, None, None)) self.descriptor.xmodule_runtime = self.system diff --git a/lms/djangoapps/xblock_user_state/__init__.py b/lms/djangoapps/xblock_user_state/__init__.py new file mode 100644 index 0000000000..e69de29bb2 diff --git a/lms/djangoapps/xblock_user_state/interface.py b/lms/djangoapps/xblock_user_state/interface.py new file mode 100644 index 0000000000..2c7254d00f --- /dev/null +++ b/lms/djangoapps/xblock_user_state/interface.py @@ -0,0 +1,255 @@ +""" +A baseclass for a generic client for accessing XBlock Scope.user_state field data. +""" + +from abc import abstractmethod + +from contracts import contract, new_contract, ContractsMeta +from opaque_keys.edx.keys import UsageKey +from xblock.fields import Scope, ScopeBase + +new_contract('UsageKey', UsageKey) + + +class XBlockUserStateClient(object): + """ + First stab at an interface for accessing XBlock User State. This will have + use StudentModule as a backing store in the default case. + + Scope/Goals: + 1. Mediate access to all student-specific state stored by XBlocks. + a. This includes "preferences" and "user_info" (i.e. UserScope.ONE) + b. This includes XBlock Asides. + c. This may later include user_state_summary (i.e. UserScope.ALL). + d. This may include group state in the future. + e. This may include other key types + UserScope.ONE (e.g. Definition) + 2. Assume network service semantics. + At some point, this will probably be calling out to an external service. + Even if it doesn't, we want to be able to implement circuit breakers, so + that a failure in StudentModule doesn't bring down the whole site. + This also implies that the client is running as a user, and whatever is + backing it is smart enough to do authorization checks. + 3. This does not yet cover export-related functionality. + + Open Questions: + 1. Is it sufficient to just send the block_key in and extract course + + version info from it? + 2. Do we want to use the username as the identifier? Privacy implications? + Ease of debugging? + 3. Would a get_many_by_type() be useful? + """ + + __metaclass__ = ContractsMeta + + class ServiceUnavailable(Exception): + """ + This error is raised if the service backing this client is currently unavailable. + """ + pass + + class PermissionDenied(Exception): + """ + This error is raised if the caller is not allowed to access the requested data. + """ + pass + + class DoesNotExist(Exception): + """ + This error is raised if the caller has requested data that does not exist. + """ + pass + + @contract( + username="basestring", + block_key=UsageKey, + scope=ScopeBase, + fields="seq(basestring)|set(basestring)|None", + returns="dict(basestring: *)" + ) + def get(self, username, block_key, scope=Scope.user_state, fields=None): + """ + Retrieve the stored XBlock state for a single xblock usage. + + Arguments: + username: The name of the user whose state should be retrieved + block_key (UsageKey): The UsageKey identifying which xblock state to load. + scope (Scope): The scope to load data from + fields: A list of field values to retrieve. If None, retrieve all stored fields. + + Returns + dict: A dictionary mapping field names to values + """ + return next(self.get_many(username, [block_key], scope, fields=fields))[1] + + @contract( + username="basestring", + block_key=UsageKey, + state="dict(basestring: *)", + scope=ScopeBase, + returns=None, + ) + def set(self, username, block_key, state, scope=Scope.user_state): + """ + Set fields for a particular XBlock. + + Arguments: + username: The name of the user whose state should be retrieved + block_key (UsageKey): The UsageKey identifying which xblock state to load. + state (dict): A dictionary mapping field names to values + scope (Scope): The scope to store data to + """ + self.set_many(username, {block_key: state}, scope) + + @contract( + username="basestring", + block_key=UsageKey, + scope=ScopeBase, + fields="seq(basestring)|set(basestring)|None", + returns=None, + ) + def delete(self, username, block_key, scope=Scope.user_state, fields=None): + """ + Delete the stored XBlock state for a single xblock usage. + + Arguments: + username: The name of the user whose state should be deleted + block_key (UsageKey): The UsageKey identifying which xblock state to delete. + scope (Scope): The scope to delete data from + fields: A list of fields to delete. If None, delete all stored fields. + """ + return self.delete_many(username, [block_key], scope, fields=fields) + + @contract( + username="basestring", + block_key=UsageKey, + scope=ScopeBase, + fields="seq(basestring)|set(basestring)|None", + returns="dict(basestring: datetime)", + ) + def get_mod_date(self, username, block_key, scope=Scope.user_state, fields=None): + """ + Get the last modification date for fields from the specified blocks. + + Arguments: + username: The name of the user whose state should queried + block_key (UsageKey): The UsageKey identifying which xblock modification dates to retrieve. + scope (Scope): The scope to retrieve from. + fields: A list of fields to query. If None, query all fields. + Specific implementations are free to return the same modification date + for all fields, if they don't store changes individually per field. + Implementations may omit fields for which data has not been stored. + + Returns: list a dict of {field_name: modified_date} for each selected field. + """ + results = self.get_mod_date_many(username, [block_key], scope, fields=fields) + return { + field: date for (_, field, date) in results + } + + @contract( + username="basestring", + block_keys="seq(UsageKey)|set(UsageKey)", + scope=ScopeBase, + fields="seq(basestring)|set(basestring)|None", + ) + @abstractmethod + def get_many(self, username, block_keys, scope=Scope.user_state, fields=None): + """ + Retrieve the stored XBlock state for a single xblock usage. + + Arguments: + username: The name of the user whose state should be retrieved + block_keys ([UsageKey]): A list of UsageKeys identifying which xblock states to load. + scope (Scope): The scope to load data from + fields: A list of field values to retrieve. If None, retrieve all stored fields. + + Yields: + (UsageKey, field_state) tuples for each specified UsageKey in block_keys. + field_state is a dict mapping field names to values. + """ + raise NotImplementedError() + + @contract( + username="basestring", + block_keys_to_state="dict(UsageKey: dict(basestring: *))", + scope=ScopeBase, + returns=None, + ) + @abstractmethod + def set_many(self, username, block_keys_to_state, scope=Scope.user_state): + """ + Set fields for a particular XBlock. + + Arguments: + username: The name of the user whose state should be retrieved + block_keys_to_state (dict): A dict mapping UsageKeys to state dicts. + Each state dict maps field names to values. These state dicts + are overlaid over the stored state. To delete fields, use + :meth:`delete` or :meth:`delete_many`. + scope (Scope): The scope to load data from + """ + raise NotImplementedError() + + @contract( + username="basestring", + block_keys="seq(UsageKey)|set(UsageKey)", + scope=ScopeBase, + fields="seq(basestring)|set(basestring)|None", + returns=None, + ) + @abstractmethod + def delete_many(self, username, block_keys, scope=Scope.user_state, fields=None): + """ + Delete the stored XBlock state for a many xblock usages. + + Arguments: + username: The name of the user whose state should be deleted + block_key (UsageKey): The UsageKey identifying which xblock state to delete. + scope (Scope): The scope to delete data from + fields: A list of fields to delete. If None, delete all stored fields. + """ + raise NotImplementedError() + + @contract( + username="basestring", + block_keys="seq(UsageKey)|set(UsageKey)", + scope=ScopeBase, + fields="seq(basestring)|set(basestring)|None", + ) + @abstractmethod + def get_mod_date_many(self, username, block_keys, scope=Scope.user_state, fields=None): + """ + Get the last modification date for fields from the specified blocks. + + Arguments: + username: The name of the user whose state should be queried + block_key (UsageKey): The UsageKey identifying which xblock modification dates to retrieve. + scope (Scope): The scope to retrieve from. + fields: A list of fields to query. If None, delete all stored fields. + Specific implementations are free to return the same modification date + for all fields, if they don't store changes individually per field. + Implementations may omit fields for which data has not been stored. + + Yields: tuples of (block, field_name, modified_date) for each selected field. + """ + raise NotImplementedError() + + def get_history(self, username, block_key, scope=Scope.user_state): + """We don't guarantee that history for many blocks will be fast.""" + raise NotImplementedError() + + def iter_all_for_block(self, block_key, scope=Scope.user_state, batch_size=None): + """ + You get no ordering guarantees. Fetching will happen in batch_size + increments. If you're using this method, you should be running in an + async task. + """ + raise NotImplementedError() + + def iter_all_for_course(self, course_key, block_type=None, scope=Scope.user_state, batch_size=None): + """ + You get no ordering guarantees. Fetching will happen in batch_size + increments. If you're using this method, you should be running in an + async task. + """ + raise NotImplementedError() diff --git a/manage.py b/manage.py index c14d386e87..266e656128 100755 --- a/manage.py +++ b/manage.py @@ -113,4 +113,5 @@ if __name__ == "__main__": from django.core.management import execute_from_command_line - execute_from_command_line([sys.argv[0]] + django_args) + sys.argv[1:] = django_args + execute_from_command_line(sys.argv) diff --git a/pavelib/utils/test/suites/nose_suite.py b/pavelib/utils/test/suites/nose_suite.py index c0597e668c..130cad4e95 100644 --- a/pavelib/utils/test/suites/nose_suite.py +++ b/pavelib/utils/test/suites/nose_suite.py @@ -120,7 +120,7 @@ class SystemTestSuite(NoseTestSuite): @property def cmd(self): cmd = ( - './manage.py {system} test --verbosity={verbosity} ' + './manage.py {system} --contracts test --verbosity={verbosity} ' '{test_id} {test_opts} --traceback --settings=test {extra} ' '--with-xunit --xunit-file={xunit_report}'.format( system=self.root, diff --git a/requirements/edx/github.txt b/requirements/edx/github.txt index f205c0a0b1..f8529ad54f 100644 --- a/requirements/edx/github.txt +++ b/requirements/edx/github.txt @@ -30,7 +30,7 @@ git+https://github.com/pmitros/pyfs.git@96e1922348bfe6d99201b9512a9ed946c87b7e0b git+https://github.com/hmarr/django-debug-toolbar-mongo.git@b0686a76f1ce3532088c4aee6e76b9abe61cc808 # Our libraries: --e git+https://github.com/edx/XBlock.git@1934a2978cdd3e2414486c74b76e3040ff1fb138#egg=XBlock +-e git+https://github.com/cpennington/XBlock.git@e9c4d441d1eaf7c9f176b873fe23e24c1152dba6#egg=XBlock -e git+https://github.com/edx/codejail.git@6b17c33a89bef0ac510926b1d7fea2748b73aadd#egg=codejail -e git+https://github.com/edx/js-test-tool.git@v0.1.6#egg=js_test_tool -e git+https://github.com/edx/event-tracking.git@0.2.0#egg=event-tracking