Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Add MAX_PAGE_SIZE setting #9107

Open
wants to merge 5 commits into
base: master
Choose a base branch
from
Open
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
8 changes: 5 additions & 3 deletions docs/api-guide/pagination.md
Original file line number Diff line number Diff line change
Expand Up @@ -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.

Expand Down
8 changes: 5 additions & 3 deletions rest_framework/checks.py
Original file line number Diff line number Diff line change
Expand Up @@ -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"
)
Expand Down
77 changes: 63 additions & 14 deletions rest_framework/pagination.py
Original file line number Diff line number Diff line change
Expand Up @@ -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

Expand All @@ -184,16 +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.
max_page_size = None

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.
"""
Comment on lines +191 to +196

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

There shoud probably be a comment explaining why this i a property to avoid removing it in a future refactor?

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
Expand Down Expand Up @@ -377,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 = None
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Can we avoid changing the style here?

max_limit = api_settings.MAX_PAGE_SIZE

Perfer minimal impact footprints for PRs.

Copy link
Contributor Author

@TheSuperiorStanislav TheSuperiorStanislav Sep 20, 2024

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

It's related to #8993 and #9107 (comment). Without it we can't properly test it, since this setting will be set during init.

Copy link
Member

@tomchristie tomchristie Sep 20, 2024

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Hrm... not sure? If you'd like to take that approach then I'd probably suggest handling #8993 separately, as a precursor to this.
Unclear to me why that'd be different to attributes we use in other cases.

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)
Expand Down Expand Up @@ -588,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'
Expand All @@ -598,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 = None

# 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)
Expand Down
1 change: 1 addition & 0 deletions rest_framework/settings.py
Original file line number Diff line number Diff line change
Expand Up @@ -65,6 +65,7 @@

# Pagination
'PAGE_SIZE': None,
"MAX_PAGE_SIZE": None,

# Filtering
'SEARCH_PARAM': 'search',
Expand Down
73 changes: 72 additions & 1 deletion tests/test_pagination.py
Original file line number Diff line number Diff line change
@@ -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
Expand Down Expand Up @@ -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.
Expand Down
9 changes: 9 additions & 0 deletions tests/test_settings.py
Original file line number Diff line number Diff line change
Expand Up @@ -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')
Expand All @@ -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):
Expand Down
Loading