Commit a2cbd166 by Calen Pennington

Merge pull request #7789 from cpennington/wrap-csm

Wrap access to CSM (inside FieldDataCache) to use the new interface
parents af69635f 122039ac
......@@ -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.
......
......@@ -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):
......
......@@ -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()
......
......@@ -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
......
......@@ -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')
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()
field_data_cache.set_score(
user_id,
descriptor.location,
grade,
max_grade,
)
# 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'
......
......@@ -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):
......
......@@ -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',
......
......@@ -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'
......
......@@ -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)
......
......@@ -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
......
......@@ -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)
......@@ -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,
......
......@@ -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
......
Markdown is supported
0% or
You are about to add 0 people to the discussion. Proceed with caution.
Finish editing this message first!
Please register or to comment