Merge pull request #11523 from edx/perf/speed-up-slow-user-role-ops

Don't blow away the role cache when updating.

cms/djangoapps/contentstore/views/tests/test_course_index.py

@@ -233,7 +233,7 @@ class TestCourseIndex(CourseTestCase):
             # delete nofications that are dismissed
             CourseRerunState.objects.get(id=rerun_state.id)
 
-        self.assertFalse(has_course_author_access(user2, rerun_course_key))
+        self.assertTrue(has_course_author_access(user2, rerun_course_key))
 
     def assert_correct_json_response(self, json_response):
         """

common/djangoapps/student/roles.py

@@ -54,6 +54,10 @@ class RoleCache(object):
             for access_role in self._roles
         )
 
+    def add_role(self, role):
+        """Adds a role to the cache."""
+        self._roles.add(role)
+
 
 class AccessRole(object):
     """
@@ -62,7 +66,7 @@ class AccessRole(object):
     __metaclass__ = ABCMeta
 
     @abstractmethod
-    def has_user(self, user):
+    def has_user(self, user, refresh=True):
         """
         Return whether the supplied django user has access to this role.
         """
@@ -130,7 +134,7 @@ class RoleBase(AccessRole):
         self.course_key = course_key
         self._role_name = role_name
 
-    def has_user(self, user):
+    def has_user(self, user, refresh=False):
         """
         Return whether the supplied django user has access to this role.
         """
@@ -138,7 +142,7 @@ class RoleBase(AccessRole):
             return False
 
         # pylint: disable=protected-access
-        if not hasattr(user, '_roles'):
+        if not hasattr(user, '_roles') or refresh:
             # Cache a list of tuples identifying the particular roles that a user has
             # Stored as tuples, rather than django models, to make it cheaper to construct objects for comparison
             user._roles = RoleCache(user)
@@ -157,7 +161,8 @@ class RoleBase(AccessRole):
                 entry = CourseAccessRole(user=user, role=self._role_name, course_id=self.course_key, org=self.org)
                 entry.save()
                 if hasattr(user, '_roles'):
-                    del user._roles
+                    # pylint: disable=protected-access
+                    user._roles.add_role(entry)
 
     def remove_users(self, *users):
         """

lms/djangoapps/ccx/tests/test_views.py

@@ -42,7 +42,6 @@ from student.tests.factories import (
 
 from xmodule.x_module import XModuleMixin
 from xmodule.modulestore import ModuleStoreEnum
-from xmodule.modulestore.django import modulestore
 from xmodule.modulestore.tests.django_utils import (
     ModuleStoreTestCase,
     SharedModuleStoreTestCase,
@@ -237,7 +236,7 @@ class TestCoachDashboard(CcxTestCase, LoginEnrollmentTestCase):
 
         # assert ccx creator has role=ccx_coach
         role = CourseCcxCoachRole(course_key)
-        self.assertTrue(role.has_user(self.coach))
+        self.assertTrue(role.has_user(self.coach, refresh=True))
 
         # assert that staff and instructors of master course has staff and instructor roles on ccx
         list_staff_master_course = list_with_level(self.course, 'staff')
@@ -739,25 +738,28 @@ def patched_get_children(self, usage_key_filter=None):
 @override_settings(FIELD_OVERRIDE_PROVIDERS=(
     'ccx.overrides.CustomCoursesForEdxOverrideProvider',))
 @patch('xmodule.x_module.XModuleMixin.get_children', patched_get_children, spec=True)
-class TestCCXGrades(SharedModuleStoreTestCase, LoginEnrollmentTestCase):
+class TestCCXGrades(ModuleStoreTestCase, LoginEnrollmentTestCase):
     """
     Tests for Custom Courses views.
     """
     MODULESTORE = TEST_DATA_SPLIT_MODULESTORE
 
-    @classmethod
-    def setUpClass(cls):
-        super(TestCCXGrades, cls).setUpClass()
-        cls._course = course = CourseFactory.create(enable_ccx=True)
+    def setUp(self):
+        """
+        Set up tests
+        """
+        super(TestCCXGrades, self).setUp()
+
+        self._course = CourseFactory.create(enable_ccx=True)
 
         # Create a course outline
-        cls.mooc_start = start = datetime.datetime(
+        self.start = datetime.datetime(
             2010, 5, 12, 2, 42, tzinfo=pytz.UTC
         )
         chapter = ItemFactory.create(
-            start=start, parent=course, category='sequential'
+            start=self.start, parent=self._course, category='sequential'
         )
-        cls.sections = sections = [
+        self.sections = [
             ItemFactory.create(
                 parent=chapter,
                 category="sequential",
@@ -765,7 +767,7 @@ class TestCCXGrades(SharedModuleStoreTestCase, LoginEnrollmentTestCase):
             for _ in xrange(4)
         ]
         # making problems available at class level for possible future use in tests
-        cls.problems = [
+        self.problems = [
             [
                 ItemFactory.create(
                     parent=section,
@@ -773,15 +775,9 @@ class TestCCXGrades(SharedModuleStoreTestCase, LoginEnrollmentTestCase):
                     data=StringResponseXMLFactory().build_xml(answer='foo'),
                     metadata={'rerandomize': 'always'}
                 ) for _ in xrange(4)
-            ] for section in sections
+            ] for section in self.sections
         ]
 
-    def setUp(self):
-        """
-        Set up tests
-        """
-        super(TestCCXGrades, self).setUp()
-
         # Create instructor account
         self.coach = coach = AdminFactory.create()
         self.client.login(username=coach.username, password="test")
@@ -854,13 +850,18 @@ class TestCCXGrades(SharedModuleStoreTestCase, LoginEnrollmentTestCase):
         rows = response.content.strip().split('\r')
         headers = rows[0]
 
-        # picking first student records
-        data = dict(zip(headers.strip().split(','), rows[1].strip().split(',')))
-        self.assertNotIn('HW 04', data)
-        self.assertEqual(data['HW 01'], '0.75')
-        self.assertEqual(data['HW 02'], '0.5')
-        self.assertEqual(data['HW 03'], '0.25')
-        self.assertEqual(data['HW Avg'], '0.5')
+        records = dict()
+        for i in range(1, len(rows)):
+            data = dict(zip(headers.strip().split(','), rows[i].strip().split(',')))
+            records[data['username']] = data
+
+        student_data = records[self.student.username]  # pylint: disable=no-member
+
+        self.assertNotIn('HW 04', student_data)
+        self.assertEqual(student_data['HW 01'], '0.75')
+        self.assertEqual(student_data['HW 02'], '0.5')
+        self.assertEqual(student_data['HW 03'], '0.25')
+        self.assertEqual(student_data['HW Avg'], '0.5')
 
     @patch('courseware.views.render_to_response', intercept_renderer)
     def test_student_progress(self):