From 52ab2b1313a90aca2de7279e864049f4eeaed975 Mon Sep 17 00:00:00 2001 From: Calen Pennington Date: Tue, 26 Nov 2013 15:44:09 -0500 Subject: [PATCH] Make FieldDataCache use the user from scope_ids, rather than its pre-configured user Co-author: Ned Batchelder --- lms/djangoapps/courseware/model_data.py | 16 +++++++++++----- .../courseware/tests/test_model_data.py | 11 ++++++++--- 2 files changed, 19 insertions(+), 8 deletions(-) diff --git a/lms/djangoapps/courseware/model_data.py b/lms/djangoapps/courseware/model_data.py index da5d639bf0..fd7705affc 100644 --- a/lms/djangoapps/courseware/model_data.py +++ b/lms/djangoapps/courseware/model_data.py @@ -14,10 +14,11 @@ from .models import ( import logging from django.db import DatabaseError +from django.contrib.auth.models import User from xblock.runtime import KeyValueStore from xblock.exceptions import KeyValueMultiSaveError, InvalidScopeError -from xblock.fields import Scope +from xblock.fields import Scope, UserScope log = logging.getLogger(__name__) @@ -226,10 +227,15 @@ class FieldDataCache(object): if field_object is not None: return field_object + 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 == Scope.user_state: field_object, _ = StudentModule.objects.get_or_create( course_id=self.course_id, - student=self.user, + student=User.objects.get(id=key.user_id), module_state_key=key.block_scope_id.url(), defaults={ 'state': json.dumps({}), @@ -245,12 +251,12 @@ class FieldDataCache(object): field_object, _ = XModuleStudentPrefsField.objects.get_or_create( field_name=key.field_name, module_type=key.block_scope_id, - student=self.user, + student=User.objects.get(id=key.user_id), ) elif key.scope == Scope.user_info: field_object, _ = XModuleStudentInfoField.objects.get_or_create( field_name=key.field_name, - student=self.user, + student=User.objects.get(id=key.user_id), ) cache_key = self._cache_key_from_kvs_key(key) @@ -347,7 +353,7 @@ class DjangoKeyValueStore(KeyValueStore): # the list of successful saves saved_fields.extend([field.field_name for field in field_objects[field_object]]) except DatabaseError: - log.error('Error saving fields %r', field_objects[field_object]) + log.exception('Error saving fields %r', field_objects[field_object]) raise KeyValueMultiSaveError(saved_fields) def delete(self, key): diff --git a/lms/djangoapps/courseware/tests/test_model_data.py b/lms/djangoapps/courseware/tests/test_model_data.py index bde57c1553..2286506465 100644 --- a/lms/djangoapps/courseware/tests/test_model_data.py +++ b/lms/djangoapps/courseware/tests/test_model_data.py @@ -40,11 +40,14 @@ def mock_descriptor(fields=[]): location = partial(Location, 'i4x', 'edX', 'test_course', 'problem') course_id = 'edX/test_course/test' +# The user ids here are 1 because we make a student in the setUp functions, and +# they get an id of 1. There's an assertion in setUp to ensure that assumption +# is still true. user_state_summary_key = partial(DjangoKeyValueStore.Key, Scope.user_state_summary, None, location('def_id')) settings_key = partial(DjangoKeyValueStore.Key, Scope.settings, None, location('def_id')) -user_state_key = partial(DjangoKeyValueStore.Key, Scope.user_state, 'user', location('def_id')) -prefs_key = partial(DjangoKeyValueStore.Key, Scope.preferences, 'user', 'MockProblemModule') -user_info_key = partial(DjangoKeyValueStore.Key, Scope.user_info, 'user', None) +user_state_key = partial(DjangoKeyValueStore.Key, Scope.user_state, 1, location('def_id')) +prefs_key = partial(DjangoKeyValueStore.Key, Scope.preferences, 1, 'MockProblemModule') +user_info_key = partial(DjangoKeyValueStore.Key, Scope.user_info, 1, None) class StudentModuleFactory(cmfStudentModuleFactory): @@ -76,6 +79,7 @@ class TestStudentModuleStorage(TestCase): def setUp(self): student_module = StudentModuleFactory(state=json.dumps({'a_field': 'a_value', 'b_field': 'b_value'})) self.user = student_module.student + self.assertEqual(self.user.id, 1) # check our assumption hard-coded in the key functions above. self.field_data_cache = FieldDataCache([mock_descriptor([mock_field(Scope.user_state, 'a_field')])], course_id, self.user) self.kvs = DjangoKeyValueStore(self.field_data_cache) @@ -152,6 +156,7 @@ class TestStudentModuleStorage(TestCase): class TestMissingStudentModule(TestCase): def setUp(self): self.user = UserFactory.create(username='user') + self.assertEqual(self.user.id, 1) # check our assumption hard-coded in the key functions above. self.field_data_cache = FieldDataCache([mock_descriptor()], course_id, self.user) self.kvs = DjangoKeyValueStore(self.field_data_cache)