From a9d25af7918475b48d9486c6d8a5f6d81db2ea89 Mon Sep 17 00:00:00 2001 From: Stanislav Khlud Date: Thu, 14 Sep 2023 10:42:02 +0700 Subject: [PATCH 1/4] Add MAX_PAGE_SIZE setting --- docs/api-guide/pagination.md | 8 +++++--- rest_framework/pagination.py | 6 +++++- rest_framework/settings.py | 1 + 3 files changed, 11 insertions(+), 4 deletions(-) diff --git a/docs/api-guide/pagination.md b/docs/api-guide/pagination.md index 41887ffd8a..010a094dfe 100644 --- a/docs/api-guide/pagination.md +++ b/docs/api-guide/pagination.md @@ -24,14 +24,16 @@ Pagination can be turned off by setting the pagination class to `None`. ## Setting the pagination style -The pagination style may be set globally, using the `DEFAULT_PAGINATION_CLASS` and `PAGE_SIZE` setting keys. For example, to use the built-in limit/offset pagination, you would do something like this: +The pagination style may be set globally, using the `DEFAULT_PAGINATION_CLASS`, `PAGE_SIZE` and `MAX_PAGE_SIZE` setting keys. For example, to use the built-in limit/offset pagination, you would do something like this: REST_FRAMEWORK = { 'DEFAULT_PAGINATION_CLASS': 'rest_framework.pagination.LimitOffsetPagination', - 'PAGE_SIZE': 100 + 'PAGE_SIZE': 100, + 'MAX_PAGE_SIZE': 250, } -Note that you need to set both the pagination class, and the page size that should be used. Both `DEFAULT_PAGINATION_CLASS` and `PAGE_SIZE` are `None` by default. +Note that you need to set both the pagination class, and the page size and limit that should be used. +`DEFAULT_PAGINATION_CLASS`, `PAGE_SIZE`, `MAX_PAGE_SIZE` are `None` by default. You can also set the pagination class on an individual view by using the `pagination_class` attribute. Typically you'll want to use the same pagination style throughout your API, although you might want to vary individual aspects of the pagination, such as default or maximum page size, on a per-view basis. diff --git a/rest_framework/pagination.py b/rest_framework/pagination.py index a543ceeb50..e45799da03 100644 --- a/rest_framework/pagination.py +++ b/rest_framework/pagination.py @@ -172,6 +172,10 @@ class PageNumberPagination(BasePagination): # The default page size. # Defaults to `None`, meaning pagination is disabled. page_size = api_settings.PAGE_SIZE + # The maximum page size. + # Defaults to `None`, meaning page size is unlimited. + # It's recommended that you would set a limit to avoid api abuse. + page_size = api_settings.MAX_PAGE_SIZE django_paginator_class = DjangoPaginator @@ -382,7 +386,7 @@ class LimitOffsetPagination(BasePagination): limit_query_description = _('Number of results to return per page.') offset_query_param = 'offset' offset_query_description = _('The initial index from which to return the results.') - max_limit = None + max_limit = api_settings.MAX_PAGE_SIZE template = 'rest_framework/pagination/numbers.html' def paginate_queryset(self, queryset, request, view=None): diff --git a/rest_framework/settings.py b/rest_framework/settings.py index b0d7bacecc..ee9ee61f1c 100644 --- a/rest_framework/settings.py +++ b/rest_framework/settings.py @@ -65,6 +65,7 @@ # Pagination 'PAGE_SIZE': None, + "MAX_PAGE_SIZE": None, # Filtering 'SEARCH_PARAM': 'search', From 1ada7c7d22bc7569261a684672c1eacd4453df94 Mon Sep 17 00:00:00 2001 From: Stanislav Khlud Date: Fri, 15 Sep 2023 11:09:36 +0700 Subject: [PATCH 2/4] Update check for pagination settings, update test --- rest_framework/checks.py | 8 +++++--- tests/test_settings.py | 9 +++++++++ 2 files changed, 14 insertions(+), 3 deletions(-) diff --git a/rest_framework/checks.py b/rest_framework/checks.py index d5d77bc59b..3cfda4c93e 100644 --- a/rest_framework/checks.py +++ b/rest_framework/checks.py @@ -6,14 +6,16 @@ def pagination_system_check(app_configs, **kwargs): errors = [] # Use of default page size setting requires a default Paginator class from rest_framework.settings import api_settings - if api_settings.PAGE_SIZE and not api_settings.DEFAULT_PAGINATION_CLASS: + if ( + api_settings.PAGE_SIZE or api_settings.MAX_PAGE_SIZE + ) and not api_settings.DEFAULT_PAGINATION_CLASS: errors.append( Warning( - "You have specified a default PAGE_SIZE pagination rest_framework setting, " + "You have specified a default PAGE_SIZE pagination or MAX_PAGE_SIZE limit rest_framework setting, " "without specifying also a DEFAULT_PAGINATION_CLASS.", hint="The default for DEFAULT_PAGINATION_CLASS is None. " "In previous versions this was PageNumberPagination. " - "If you wish to define PAGE_SIZE globally whilst defining " + "If you wish to define PAGE_SIZE or MAX_PAGE_SIZE globally whilst defining " "pagination_class on a per-view basis you may silence this check.", id="rest_framework.W001" ) diff --git a/tests/test_settings.py b/tests/test_settings.py index 15624c0599..0ca51be908 100644 --- a/tests/test_settings.py +++ b/tests/test_settings.py @@ -54,6 +54,7 @@ def get_pagination_error(error_id: str): return next((error for error in errors if error.id == error_id), None) self.assertIsNone(api_settings.PAGE_SIZE) + self.assertIsNone(api_settings.MAX_PAGE_SIZE) self.assertIsNone(api_settings.DEFAULT_PAGINATION_CLASS) pagination_error = get_pagination_error('rest_framework.W001') @@ -63,11 +64,19 @@ def get_pagination_error(error_id: str): pagination_error = get_pagination_error('rest_framework.W001') self.assertIsNotNone(pagination_error) + with override_settings(REST_FRAMEWORK={'MAX_PAGE_SIZE': 10}): + pagination_error = get_pagination_error('rest_framework.W001') + self.assertIsNotNone(pagination_error) + default_pagination_class = 'rest_framework.pagination.PageNumberPagination' with override_settings(REST_FRAMEWORK={'PAGE_SIZE': 10, 'DEFAULT_PAGINATION_CLASS': default_pagination_class}): pagination_error = get_pagination_error('rest_framework.W001') self.assertIsNone(pagination_error) + with override_settings(REST_FRAMEWORK={'MAX_PAGE_SIZE': 10, 'DEFAULT_PAGINATION_CLASS': default_pagination_class}): + pagination_error = get_pagination_error('rest_framework.W001') + self.assertIsNone(pagination_error) + class TestSettingTypes(TestCase): def test_settings_consistently_coerced_to_list(self): From 91695fb3c67fa4d760b433dd9decf9c4d590a273 Mon Sep 17 00:00:00 2001 From: Stanislav Khlud Date: Fri, 15 Sep 2023 16:23:33 +0700 Subject: [PATCH 3/4] Fix setting typo Co-authored-by: Roman Gorbil --- rest_framework/pagination.py | 10 ++++------ 1 file changed, 4 insertions(+), 6 deletions(-) diff --git a/rest_framework/pagination.py b/rest_framework/pagination.py index e45799da03..ede9591ca3 100644 --- a/rest_framework/pagination.py +++ b/rest_framework/pagination.py @@ -172,10 +172,6 @@ class PageNumberPagination(BasePagination): # The default page size. # Defaults to `None`, meaning pagination is disabled. page_size = api_settings.PAGE_SIZE - # The maximum page size. - # Defaults to `None`, meaning page size is unlimited. - # It's recommended that you would set a limit to avoid api abuse. - page_size = api_settings.MAX_PAGE_SIZE django_paginator_class = DjangoPaginator @@ -190,7 +186,9 @@ class PageNumberPagination(BasePagination): # Set to an integer to limit the maximum page size the client may request. # Only relevant if 'page_size_query_param' has also been set. - max_page_size = None + # Defaults to `None`, meaning page size is unlimited. + # It's recommended that you would set a limit to avoid api abuse. + max_page_size = api_settings.MAX_PAGE_SIZE last_page_strings = ('last',) @@ -604,7 +602,7 @@ class CursorPagination(BasePagination): # Set to an integer to limit the maximum page size the client may request. # Only relevant if 'page_size_query_param' has also been set. - max_page_size = None + max_page_size = api_settings.MAX_PAGE_SIZE # The offset in the cursor is used in situations where we have a # nearly-unique index. (Eg millisecond precision creation timestamps) From b51141cb6958f356fca8c65efdcab8ffb43fa571 Mon Sep 17 00:00:00 2001 From: Stanislav Khlud Date: Wed, 21 Aug 2024 09:01:54 +0700 Subject: [PATCH 4/4] Move page settings to property --- rest_framework/pagination.py | 79 ++++++++++++++++++++++++++++-------- tests/test_pagination.py | 73 ++++++++++++++++++++++++++++++++- 2 files changed, 135 insertions(+), 17 deletions(-) diff --git a/rest_framework/pagination.py b/rest_framework/pagination.py index ede9591ca3..47b44ec801 100644 --- a/rest_framework/pagination.py +++ b/rest_framework/pagination.py @@ -169,9 +169,6 @@ class PageNumberPagination(BasePagination): http://api.example.org/accounts/?page=4 http://api.example.org/accounts/?page=4&page_size=100 """ - # The default page size. - # Defaults to `None`, meaning pagination is disabled. - page_size = api_settings.PAGE_SIZE django_paginator_class = DjangoPaginator @@ -184,18 +181,33 @@ class PageNumberPagination(BasePagination): page_size_query_param = None page_size_query_description = _('Number of results to return per page.') - # Set to an integer to limit the maximum page size the client may request. - # Only relevant if 'page_size_query_param' has also been set. - # Defaults to `None`, meaning page size is unlimited. - # It's recommended that you would set a limit to avoid api abuse. - max_page_size = api_settings.MAX_PAGE_SIZE - last_page_strings = ('last',) template = 'rest_framework/pagination/numbers.html' invalid_page_message = _('Invalid page.') + @property + def page_size(self) -> int: + """Get default page size. + + Defaults to `None`, meaning pagination is disabled. + + """ + return api_settings.PAGE_SIZE + + @property + def max_page_size(self) -> int: + """Limit page size. + + Set to an integer to limit the maximum page size the client may request. + Only relevant if 'page_size_query_param' has also been set. + Defaults to `None`, meaning page size is unlimited. + It's recommended that you would set a limit to avoid api abuse. + + """ + return api_settings.MAX_PAGE_SIZE + def paginate_queryset(self, queryset, request, view=None): """ Paginate a queryset if required, either returning a @@ -379,14 +391,33 @@ class LimitOffsetPagination(BasePagination): http://api.example.org/accounts/?limit=100 http://api.example.org/accounts/?offset=400&limit=100 """ - default_limit = api_settings.PAGE_SIZE limit_query_param = 'limit' limit_query_description = _('Number of results to return per page.') offset_query_param = 'offset' offset_query_description = _('The initial index from which to return the results.') - max_limit = api_settings.MAX_PAGE_SIZE template = 'rest_framework/pagination/numbers.html' + @property + def max_limit(self) -> int: + """Limit maximum page size. + + Set to an integer to limit the maximum page size the client may request. + Only relevant if 'page_size_query_param' has also been set. + Defaults to `None`, meaning page size is unlimited. + It's recommended that you would set a limit to avoid api abuse. + + """ + return api_settings.MAX_PAGE_SIZE + + @property + def default_limit(self) -> int: + """Get default page size. + + Defaults to `None`, meaning pagination is disabled. + + """ + return api_settings.PAGE_SIZE + def paginate_queryset(self, queryset, request, view=None): self.request = request self.limit = self.get_limit(request) @@ -590,7 +621,6 @@ class CursorPagination(BasePagination): """ cursor_query_param = 'cursor' cursor_query_description = _('The pagination cursor value.') - page_size = api_settings.PAGE_SIZE invalid_cursor_message = _('Invalid cursor') ordering = '-created' template = 'rest_framework/pagination/previous_and_next.html' @@ -600,16 +630,33 @@ class CursorPagination(BasePagination): page_size_query_param = None page_size_query_description = _('Number of results to return per page.') - # Set to an integer to limit the maximum page size the client may request. - # Only relevant if 'page_size_query_param' has also been set. - max_page_size = api_settings.MAX_PAGE_SIZE - # The offset in the cursor is used in situations where we have a # nearly-unique index. (Eg millisecond precision creation timestamps) # We guard against malicious users attempting to cause expensive database # queries, by having a hard cap on the maximum possible size of the offset. offset_cutoff = 1000 + @property + def page_size(self) -> int: + """Get default page size. + + Defaults to `None`, meaning pagination is disabled. + + """ + return api_settings.PAGE_SIZE + + @property + def max_page_size(self) -> int: + """Limit page size. + + Set to an integer to limit the maximum page size the client may request. + Only relevant if 'page_size_query_param' has also been set. + Defaults to `None`, meaning page size is unlimited. + It's recommended that you would set a limit to avoid api abuse. + + """ + return api_settings.MAX_PAGE_SIZE + def paginate_queryset(self, queryset, request, view=None): self.request = request self.page_size = self.get_page_size(request) diff --git a/tests/test_pagination.py b/tests/test_pagination.py index 02d443ade0..dcf924bc85 100644 --- a/tests/test_pagination.py +++ b/tests/test_pagination.py @@ -1,7 +1,7 @@ import pytest from django.core.paginator import Paginator as DjangoPaginator from django.db import models -from django.test import TestCase +from django.test import TestCase, override_settings from rest_framework import ( exceptions, filters, generics, pagination, serializers, status @@ -135,6 +135,77 @@ def test_404_not_found_for_invalid_page(self): } +class TestPaginationSettingsIntegration: + """ + Integration tests for pagination settings. + """ + + def setup_method(self): + class PassThroughSerializer(serializers.BaseSerializer): + def to_representation(self, item): + return item + + class EvenItemsOnly(filters.BaseFilterBackend): + def filter_queryset(self, request, queryset, view): + return [item for item in queryset if item % 2 == 0] + + class BasicPagination(pagination.PageNumberPagination): + page_size_query_param = 'page_size' + + self.view = generics.ListAPIView.as_view( + serializer_class=PassThroughSerializer, + queryset=range(1, 101), + filter_backends=[EvenItemsOnly], + pagination_class=BasicPagination + ) + + @override_settings( + REST_FRAMEWORK={ + "MAX_PAGE_SIZE": 20, + "PAGE_SIZE": 5, + } + ) + def test_setting_page_size_over_maximum(self): + """ + When page_size parameter exceeds maximum allowable, + then it should be capped to the maximum. + """ + request = factory.get('/', {'page_size': 1000}) + response = self.view(request) + assert response.status_code == status.HTTP_200_OK + assert len(response.data["results"]) == 20, response.data + assert response.data == { + 'results': [ + 2, 4, 6, 8, 10, 12, 14, 16, 18, 20, + 22, 24, 26, 28, 30, 32, 34, 36, 38, 40 + ], + 'previous': None, + 'next': 'http://testserver/?page=2&page_size=1000', + 'count': 50 + } + + @override_settings( + REST_FRAMEWORK={ + "MAX_PAGE_SIZE": 20, + "PAGE_SIZE": 5, + } + ) + def test_setting_page_size_to_zero(self): + """ + When page_size parameter is invalid it should return to the default. + """ + request = factory.get('/', {'page_size': 0}) + response = self.view(request) + assert response.status_code == status.HTTP_200_OK + assert len(response.data["results"]) == 5, response.data + assert response.data == { + 'results': [2, 4, 6, 8, 10], + 'previous': None, + 'next': 'http://testserver/?page=2&page_size=0', + 'count': 50 + } + + class TestPaginationDisabledIntegration: """ Integration tests for disabled pagination.