Merge pull request #18362 from edx/waheed/LEARNER-5527-throttle-course-list-api

Rate limit course list API.

lms/djangoapps/course_api/__init__.py

@@ -1 +1,7 @@
 """ Course API """
+from openedx.core.djangoapps.waffle_utils import (WaffleSwitch, WaffleSwitchNamespace)
+
+WAFFLE_SWITCH_NAMESPACE = WaffleSwitchNamespace(name='course_list_api_rate_limit')
+
+USE_RATE_LIMIT_2_FOR_COURSE_LIST_API = WaffleSwitch(WAFFLE_SWITCH_NAMESPACE, 'rate_limit_2')
+USE_RATE_LIMIT_10_FOR_COURSE_LIST_API = WaffleSwitch(WAFFLE_SWITCH_NAMESPACE, 'rate_limit_10')

lms/djangoapps/course_api/tests/test_views.py

@@ -1,8 +1,10 @@
 """
 Tests for Course API views.
 """
+import ddt
 from hashlib import md5
 
+from django.core.exceptions import ImproperlyConfigured
 from django.urls import reverse
 from django.test import RequestFactory
 from django.test.utils import override_settings
@@ -12,8 +14,9 @@ from search.tests.tests import TEST_INDEX_NAME
 from search.tests.utils import SearcherMixin
 
 from xmodule.modulestore.tests.django_utils import ModuleStoreTestCase, SharedModuleStoreTestCase
+from waffle.testutils import override_switch
 
-from ..views import CourseDetailView
+from ..views import CourseDetailView, CourseListUserThrottle
 from .mixins import TEST_PASSWORD, CourseApiFactoryMixin
 
 
@@ -53,6 +56,7 @@ class CourseApiTestViewMixin(CourseApiFactoryMixin):
 
 
 @attr(shard=9)
+@ddt.ddt
 class CourseListViewTestCase(CourseApiTestViewMixin, SharedModuleStoreTestCase):
     """
     Test responses returned from CourseListView.
@@ -100,6 +104,38 @@ class CourseListViewTestCase(CourseApiTestViewMixin, SharedModuleStoreTestCase):
         self.client.logout()
         self.verify_response()
 
+    def assert_throttle_configured_correctly(self, user_scope, throws_exception, expected_rate):
+        """Helper to determine throttle configuration is correctly set."""
+        throttle = CourseListUserThrottle()
+        throttle.check_for_switches()
+        throttle.scope = user_scope
+        try:
+            rate_limit, __ = throttle.parse_rate(throttle.get_rate())
+            self.assertEqual(rate_limit, expected_rate)
+            self.assertFalse(throws_exception)
+        except ImproperlyConfigured:
+            self.assertTrue(throws_exception)
+
+    @ddt.data(('staff', False, 40), ('user', False, 20), ('unknown', True, None))
+    @ddt.unpack
+    def test_throttle_rate_default(self, user_scope, throws_exception, expected_rate):
+        """ Make sure throttle rate default is set correctly for different user scopes. """
+        self.assert_throttle_configured_correctly(user_scope, throws_exception, expected_rate)
+
+    @ddt.data(('staff', False, 10), ('user', False, 2), ('unknown', True, None))
+    @ddt.unpack
+    @override_switch('course_list_api_rate_limit.rate_limit_2', active=True)
+    def test_throttle_rate_2(self, user_scope, throws_exception, expected_rate):
+        """ Make sure throttle rate 2 is set correctly for different user scopes. """
+        self.assert_throttle_configured_correctly(user_scope, throws_exception, expected_rate)
+
+    @ddt.data(('staff', False, 20), ('user', False, 10), ('unknown', True, None))
+    @ddt.unpack
+    @override_switch('course_list_api_rate_limit.rate_limit_10', active=True)
+    def test_throttle_rate_20(self, user_scope, throws_exception, expected_rate):
+        """ Make sure throttle rate 20 is set correctly for different user scopes. """
+        self.assert_throttle_configured_correctly(user_scope, throws_exception, expected_rate)
+
 
 @attr(shard=9)
 class CourseListViewTestCaseMultipleCourses(CourseApiTestViewMixin, ModuleStoreTestCase):

lms/djangoapps/course_api/views.py

@@ -6,10 +6,12 @@ import search
 from django.conf import settings
 from django.core.exceptions import ValidationError
 from rest_framework.generics import ListAPIView, RetrieveAPIView
+from rest_framework.throttling import UserRateThrottle
 
 from edx_rest_framework_extensions.paginators import NamespacedPageNumberPagination
 from openedx.core.lib.api.view_utils import DeveloperErrorViewMixin, view_auth_classes
 
+from . import USE_RATE_LIMIT_2_FOR_COURSE_LIST_API, USE_RATE_LIMIT_10_FOR_COURSE_LIST_API
 from .api import course_detail, list_courses
 from .forms import CourseDetailGetForm, CourseListGetForm
 from .serializers import CourseDetailSerializer, CourseSerializer
@@ -123,6 +125,41 @@ class CourseDetailView(DeveloperErrorViewMixin, RetrieveAPIView):
         )
 
 
+class CourseListUserThrottle(UserRateThrottle):
+    """Limit the number of requests users can make to the course list API."""
+    # The course list endpoint is likely being inefficient with how it's querying
+    # various parts of the code and can take courseware down, it needs to be rate
+    # limited until optimized. LEARNER-5527
+
+    THROTTLE_RATES = {
+        'user': '20/minute',
+        'staff': '40/minute',
+    }
+
+    def check_for_switches(self):
+        if USE_RATE_LIMIT_2_FOR_COURSE_LIST_API.is_enabled():
+            self.THROTTLE_RATES = {
+                'user': '2/minute',
+                'staff': '10/minute',
+            }
+        elif USE_RATE_LIMIT_10_FOR_COURSE_LIST_API.is_enabled():
+            self.THROTTLE_RATES = {
+                'user': '10/minute',
+                'staff': '20/minute',
+            }
+
+    def allow_request(self, request, view):
+        self.check_for_switches()
+        # Use a special scope for staff to allow for a separate throttle rate
+        user = request.user
+        if user.is_authenticated and (user.is_staff or user.is_superuser):
+            self.scope = 'staff'
+            self.rate = self.get_rate()
+            self.num_requests, self.duration = self.parse_rate(self.rate)
+
+        return super(CourseListUserThrottle, self).allow_request(request, view)
+
+
 @view_auth_classes(is_authenticated=False)
 class CourseListView(DeveloperErrorViewMixin, ListAPIView):
     """
@@ -196,6 +233,7 @@ class CourseListView(DeveloperErrorViewMixin, ListAPIView):
 
     pagination_class = NamespacedPageNumberPagination
     serializer_class = CourseSerializer
+    throttle_classes = CourseListUserThrottle,
 
     # Return all the results, 10K is the maximum allowed value for ElasticSearch.
     # We should use 0 after upgrading to 1.1+: