From 53edd37df5aa0ac29dbe7824db2e33da1d901f98 Mon Sep 17 00:00:00 2001 From: Tom Christie Date: Thu, 15 Jan 2015 21:07:05 +0000 Subject: [PATCH] Tests for LimitOffsetPagination --- docs/api-guide/pagination.md | 2 +- rest_framework/pagination.py | 57 ++++++++--------- tests/test_pagination.py | 117 ++++++++++++++++++++++++++++++++++- 3 files changed, 144 insertions(+), 32 deletions(-) diff --git a/docs/api-guide/pagination.md b/docs/api-guide/pagination.md index ba71a3032..8ab2edd53 100644 --- a/docs/api-guide/pagination.md +++ b/docs/api-guide/pagination.md @@ -63,7 +63,7 @@ Or apply the style globally, using the `DEFAULT_PAGINATION_CLASS` settings key. # Custom pagination styles -To create a custom pagination serializer class you should subclass `pagination.BasePagination` and override the `paginate_queryset(self, queryset, request, view)` and `get_paginated_response(self, data)` methods: +To create a custom pagination serializer class you should subclass `pagination.BasePagination` and override the `paginate_queryset(self, queryset, request, view=None)` and `get_paginated_response(self, data)` methods: * The `paginate_queryset` method is passed the initial queryset and should return an iterable object that contains only the data in the requested page. * The `get_paginated_response` method is passed the serialized page data and should return a `Response` instance. diff --git a/rest_framework/pagination.py b/rest_framework/pagination.py index 61b8e07ac..0dac56830 100644 --- a/rest_framework/pagination.py +++ b/rest_framework/pagination.py @@ -44,7 +44,7 @@ def _get_count(queryset): """ try: return queryset.count() - except AttributeError: + except (AttributeError, TypeError): return len(queryset) @@ -111,12 +111,7 @@ def _get_page_links(page_numbers, current, url_func): page_links = [] for page_number in page_numbers: if page_number is None: - page_link = PageLink( - url=None, - number=None, - is_active=False, - is_break=True - ) + page_link = PAGE_BREAK else: page_link = PageLink( url=url_func(page_number), @@ -130,11 +125,13 @@ def _get_page_links(page_numbers, current, url_func): PageLink = namedtuple('PageLink', ['url', 'number', 'is_active', 'is_break']) +PAGE_BREAK = PageLink(url=None, number=None, is_active=False, is_break=True) + class BasePagination(object): display_page_controls = False - def paginate_queryset(self, queryset, request, view): + def paginate_queryset(self, queryset, request, view=None): raise NotImplemented('paginate_queryset() must be implemented.') def get_paginated_response(self, data): @@ -167,9 +164,11 @@ class PageNumberPagination(BasePagination): # Only relevant if 'paginate_by_param' has also been set. max_paginate_by = api_settings.MAX_PAGINATE_BY + last_page_strings = ('last',) + template = 'rest_framework/pagination/numbers.html' - def paginate_queryset(self, queryset, request, view): + def paginate_queryset(self, queryset, request, view=None): """ Paginate a queryset if required, either returning a page object, or `None` if pagination is not configured for this view. @@ -186,18 +185,9 @@ class PageNumberPagination(BasePagination): return None paginator = DjangoPaginator(queryset, page_size) - page_string = request.query_params.get(self.page_query_param, 1) - try: - page_number = paginator.validate_number(page_string) - except InvalidPage: - if page_string == 'last': - page_number = paginator.num_pages - else: - msg = _( - 'Choose a valid page number. Page numbers must be a ' - 'whole number, or must be the string "last".' - ) - raise NotFound(msg) + page_number = request.query_params.get(self.page_query_param, 1) + if page_number in self.last_page_strings: + page_number = paginator.num_pages try: self.page = paginator.page(page_number) @@ -210,6 +200,7 @@ class PageNumberPagination(BasePagination): if paginator.count > 1: # The browsable API should display pagination controls. self.display_page_controls = True + self.request = request return self.page @@ -249,7 +240,7 @@ class PageNumberPagination(BasePagination): return remove_query_param(url, self.page_query_param) return replace_query_param(url, self.page_query_param, page_number) - def to_html(self): + def get_html_context(self): base_url = self.request.build_absolute_uri() def page_number_to_url(page_number): @@ -263,12 +254,15 @@ class PageNumberPagination(BasePagination): page_numbers = _get_displayed_page_numbers(current, final) page_links = _get_page_links(page_numbers, current, page_number_to_url) - template = loader.get_template(self.template) - context = Context({ + return { 'previous_url': self.get_previous_link(), 'next_url': self.get_next_link(), 'page_links': page_links - }) + } + + def to_html(self): + template = loader.get_template(self.template) + context = Context(self.get_html_context()) return template.render(context) @@ -286,7 +280,7 @@ class LimitOffsetPagination(BasePagination): template = 'rest_framework/pagination/numbers.html' - def paginate_queryset(self, queryset, request, view): + def paginate_queryset(self, queryset, request, view=None): self.limit = self.get_limit(request) self.offset = self.get_offset(request) self.count = _get_count(queryset) @@ -343,7 +337,7 @@ class LimitOffsetPagination(BasePagination): offset = self.offset - self.limit return replace_query_param(url, self.offset_query_param, offset) - def to_html(self): + def get_html_context(self): base_url = self.request.build_absolute_uri() current = _divide_with_ceil(self.offset, self.limit) + 1 final = _divide_with_ceil(self.count, self.limit) @@ -358,10 +352,13 @@ class LimitOffsetPagination(BasePagination): page_numbers = _get_displayed_page_numbers(current, final) page_links = _get_page_links(page_numbers, current, page_number_to_url) - template = loader.get_template(self.template) - context = Context({ + return { 'previous_url': self.get_previous_link(), 'next_url': self.get_next_link(), 'page_links': page_links - }) + } + + def to_html(self): + template = loader.get_template(self.template) + context = Context(self.get_html_context()) return template.render(context) diff --git a/tests/test_pagination.py b/tests/test_pagination.py index d410cd5eb..32fe7a66f 100644 --- a/tests/test_pagination.py +++ b/tests/test_pagination.py @@ -3,8 +3,10 @@ import datetime from decimal import Decimal from django.test import TestCase from django.utils import unittest -from rest_framework import generics, serializers, status, filters +from rest_framework import generics, pagination, serializers, status, filters from rest_framework.compat import django_filters +from rest_framework.request import Request +from rest_framework.pagination import PageLink, PAGE_BREAK from rest_framework.test import APIRequestFactory from .models import BasicModel, FilterableItem @@ -337,3 +339,116 @@ class TestMaxPaginateByParam(TestCase): request = factory.get('/') response = self.view(request).render() self.assertEqual(response.data['results'], self.data[:3]) + + +class TestLimitOffset: + def setup(self): + self.pagination = pagination.LimitOffsetPagination() + self.queryset = range(1, 101) + + def paginate_queryset(self, request): + return self.pagination.paginate_queryset(self.queryset, request) + + def get_paginated_content(self, queryset): + response = self.pagination.get_paginated_response(queryset) + return response.data + + def get_html_context(self): + return self.pagination.get_html_context() + + def test_no_offset(self): + request = Request(factory.get('/', {'limit': 5})) + queryset = self.paginate_queryset(request) + content = self.get_paginated_content(queryset) + context = self.get_html_context() + assert queryset == [1, 2, 3, 4, 5] + assert content == { + 'results': [1, 2, 3, 4, 5], + 'previous': None, + 'next': 'http://testserver/?limit=5&offset=5', + 'count': 100 + } + assert context == { + 'previous_url': None, + 'next_url': 'http://testserver/?limit=5&offset=5', + 'page_links': [ + PageLink('http://testserver/?limit=5', 1, True, False), + PageLink('http://testserver/?limit=5&offset=5', 2, False, False), + PageLink('http://testserver/?limit=5&offset=10', 3, False, False), + PAGE_BREAK, + PageLink('http://testserver/?limit=5&offset=95', 20, False, False), + ] + } + + def test_first_offset(self): + request = Request(factory.get('/', {'limit': 5, 'offset': 5})) + queryset = self.paginate_queryset(request) + content = self.get_paginated_content(queryset) + context = self.get_html_context() + assert queryset == [6, 7, 8, 9, 10] + assert content == { + 'results': [6, 7, 8, 9, 10], + 'previous': 'http://testserver/?limit=5', + 'next': 'http://testserver/?limit=5&offset=10', + 'count': 100 + } + assert context == { + 'previous_url': 'http://testserver/?limit=5', + 'next_url': 'http://testserver/?limit=5&offset=10', + 'page_links': [ + PageLink('http://testserver/?limit=5', 1, False, False), + PageLink('http://testserver/?limit=5&offset=5', 2, True, False), + PageLink('http://testserver/?limit=5&offset=10', 3, False, False), + PAGE_BREAK, + PageLink('http://testserver/?limit=5&offset=95', 20, False, False), + ] + } + + def test_middle_offset(self): + request = Request(factory.get('/', {'limit': 5, 'offset': 10})) + queryset = self.paginate_queryset(request) + content = self.get_paginated_content(queryset) + context = self.get_html_context() + assert queryset == [11, 12, 13, 14, 15] + assert content == { + 'results': [11, 12, 13, 14, 15], + 'previous': 'http://testserver/?limit=5&offset=5', + 'next': 'http://testserver/?limit=5&offset=15', + 'count': 100 + } + assert context == { + 'previous_url': 'http://testserver/?limit=5&offset=5', + 'next_url': 'http://testserver/?limit=5&offset=15', + 'page_links': [ + PageLink('http://testserver/?limit=5', 1, False, False), + PageLink('http://testserver/?limit=5&offset=5', 2, False, False), + PageLink('http://testserver/?limit=5&offset=10', 3, True, False), + PageLink('http://testserver/?limit=5&offset=15', 4, False, False), + PAGE_BREAK, + PageLink('http://testserver/?limit=5&offset=95', 20, False, False), + ] + } + + def test_ending_offset(self): + request = Request(factory.get('/', {'limit': 5, 'offset': 95})) + queryset = self.paginate_queryset(request) + content = self.get_paginated_content(queryset) + context = self.get_html_context() + assert queryset == [96, 97, 98, 99, 100] + assert content == { + 'results': [96, 97, 98, 99, 100], + 'previous': 'http://testserver/?limit=5&offset=90', + 'next': None, + 'count': 100 + } + assert context == { + 'previous_url': 'http://testserver/?limit=5&offset=90', + 'next_url': None, + 'page_links': [ + PageLink('http://testserver/?limit=5', 1, False, False), + PAGE_BREAK, + PageLink('http://testserver/?limit=5&offset=85', 18, False, False), + PageLink('http://testserver/?limit=5&offset=90', 19, False, False), + PageLink('http://testserver/?limit=5&offset=95', 20, True, False), + ] + }