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

Rate limit course list API.
This commit is contained in:
Waheed Ahmed
2018-06-12 16:59:27 +05:00
committed by GitHub
2 changed files with 35 additions and 1 deletions

View File

@@ -7,13 +7,14 @@ from django.urls import reverse
from django.test import RequestFactory
from django.test.utils import override_settings
from nose.plugins.attrib import attr
from rest_framework import status
from search.tests.test_course_discovery import DemoCourse
from search.tests.tests import TEST_INDEX_NAME
from search.tests.utils import SearcherMixin
from xmodule.modulestore.tests.django_utils import ModuleStoreTestCase, SharedModuleStoreTestCase
from ..views import CourseDetailView
from ..views import CourseDetailView, CourseListUserThrottle
from .mixins import TEST_PASSWORD, CourseApiFactoryMixin
@@ -100,6 +101,15 @@ class CourseListViewTestCase(CourseApiTestViewMixin, SharedModuleStoreTestCase):
self.client.logout()
self.verify_response()
def test_throttle_for_user(self):
"""Make sure a user requests do not exceed the maximum number of requests"""
throttle = CourseListUserThrottle()
rate_limit, __ = throttle.parse_rate(throttle.rate)
self.setup_user(self.honor_user)
for attempt in xrange(rate_limit + 2):
expected_status = status.HTTP_429_TOO_MANY_REQUESTS if attempt >= rate_limit else status.HTTP_200_OK
self.verify_response(expected_status_code=expected_status, params={'username': self.honor_user.username})
@attr(shard=9)
class CourseListViewTestCaseMultipleCourses(CourseApiTestViewMixin, ModuleStoreTestCase):

View File

@@ -6,6 +6,7 @@ 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
@@ -123,6 +124,28 @@ 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': '2/minute',
'staff': '10/minute',
}
def allow_request(self, request, view):
# 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 +219,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+: