Merge pull request #19998 from cpennington/revert-aggressive-prefetching

Revert "Improve the performance of access checking for a specific use…

common/djangoapps/django_comment_common/models.py

@@ -121,14 +121,7 @@ class Role(models.Model):
         """
         Returns True if the user has one of the given roles for the given course
         """
-        if 'roles' in getattr(user, '_prefetched_objects_cache', {}):
-            # Don't blow up the prefetch cache
-            return any(
-                role.course_id == course_id and role.name in role_names
-                for role in user.roles.all()
-            )
-        else:
-            return user.roles.filter(course_id=course_id, name__in=role_names).exists()
+        return Role.objects.filter(course_id=course_id, name__in=role_names, users=user).exists()
 
 
 class Permission(models.Model):

common/djangoapps/student/models.py

@@ -1195,19 +1195,13 @@ class CourseEnrollment(models.Model):
         if user.is_anonymous:
             return None
         try:
-            if 'courseenrollment' in getattr(user, '_prefetched_objects_cache', {}):
-                for enrollment in user.courseenrollment_set.all():
-                    if enrollment.course_id == course_key:
-                        return enrollment
-                return None
-            else:
-                query = cls.objects
-                if select_related is not None:
-                    query = query.select_related(*select_related)
-                return query.get(
-                    user=user,
-                    course_id=course_key
-                )
+            query = cls.objects
+            if select_related is not None:
+                query = query.select_related(*select_related)
+            return query.get(
+                user=user,
+                course_id=course_key
+            )
         except cls.DoesNotExist:
             return None
 

lms/djangoapps/course_api/tests/test_views.py

@@ -277,7 +277,6 @@ class CourseDetailViewTestCase(CourseApiTestViewMixin, SharedModuleStoreTestCase
 })
 @override_settings(SEARCH_ENGINE="search.tests.mock_search_engine.MockSearchEngine")
 @override_settings(COURSEWARE_INDEX_NAME=TEST_INDEX_NAME)
-@ddt.ddt
 class CourseListSearchViewTest(CourseApiTestViewMixin, ModuleStoreTestCase, SearcherMixin):
     """
     Tests the search functionality of the courses API.
@@ -356,8 +355,7 @@ class CourseListSearchViewTest(CourseApiTestViewMixin, ModuleStoreTestCase, Sear
         self.assertEqual(res.data['pagination']['count'], 3)
         self.assertEqual(len(res.data['results']), 1)  # Should return a single course
 
-    @ddt.data(True, False)
-    def test_too_many_courses(self, with_username):
+    def test_too_many_courses(self):
         """
         Test that search results are limited to 100 courses, and that they don't
         blow up the database.
@@ -396,14 +394,8 @@ class CourseListSearchViewTest(CourseApiTestViewMixin, ModuleStoreTestCase, Sear
 
         for page in range(1, 12):
             RequestCache.clear_all_namespaces()
-            expected_query_count = query_counts[page - 1]
-            if with_username:
-                expected_query_count += 4
-            with self.assertNumQueries(expected_query_count):
-                params = {'page': page, 'page_size': 30}
-                if with_username:
-                    params['username'] = self.audit_user.username
-                response = self.verify_response(params=params)
+            with self.assertNumQueries(query_counts[page - 1]):
+                response = self.verify_response(params={'page': page, 'page_size': 30})
 
                 self.assertIn('results', response.data)
                 self.assertEqual(response.data['pagination']['count'], 303)

lms/djangoapps/courseware/courses.py

@@ -25,7 +25,7 @@ from courseware.model_data import FieldDataCache
 from courseware.module_render import get_module
 from course_modes.models import CourseMode
 from django.conf import settings
-from django.db.models import Prefetch, prefetch_related_objects
+from django.db.models import Prefetch
 from django.urls import reverse
 from django.http import Http404, QueryDict
 from enrollment.api import get_course_enrollment_details
@@ -474,8 +474,7 @@ def get_courses(user, org=None, filter_=None):
         'COURSE_CATALOG_VISIBILITY_PERMISSION',
         settings.COURSE_CATALOG_VISIBILITY_PERMISSION
     )
-    if user.is_authenticated:
-        prefetch_related_objects([user], 'roles', 'courseenrollment_set', 'experimentdata_set')
+
     return LazySequence(
         (c for c in courses if has_access(user, permission_name, c)),
         est_len=courses.count()

openedx/core/djangoapps/config_model_utils/utils.py

@@ -12,24 +12,14 @@ def is_in_holdback(user):
     """
     in_holdback = False
     if user and user.is_authenticated:
-        if 'experimentdata' in getattr(user, '_prefetched_objects_cache', {}):
-            for experiment_data in user.experimentdata_set.all():
-                if (
-                    experiment_data.experiment_id == EXPERIMENT_ID and
-                    experiment_data.key == EXPERIMENT_DATA_HOLDBACK_KEY
-                ):
-                    in_holdback = experiment_data.value == 'True'
-                    break
-        else:
-            try:
-                holdback_value = ExperimentData.objects.get(
-                    user=user,
-                    experiment_id=EXPERIMENT_ID,
-                    key=EXPERIMENT_DATA_HOLDBACK_KEY,
-                ).value
-            except ExperimentData.DoesNotExist:
-                pass
-            else:
-                in_holdback = holdback_value == 'True'
+        try:
+            holdback_value = ExperimentData.objects.get(
+                user=user,
+                experiment_id=EXPERIMENT_ID,
+                key=EXPERIMENT_DATA_HOLDBACK_KEY,
+            ).value
+            in_holdback = holdback_value == 'True'
+        except ExperimentData.DoesNotExist:
+            pass
 
     return in_holdback