Make FieldDataCache use the user from scope_ids, rather than its pre-configured user

Co-author: Ned Batchelder <ned@edx.org>

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):

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)