Tests for LimitOffsetPagination

3 files changed, 144 insertions, 32 deletions

diff --git a/docs/api-guide/pagination.md b/docs/api-guide/pagination.md
index ba71a303..8ab2edd5 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 61b8e07a..0dac5683 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 d410cd5e..32fe7a66 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),
+            ]
+        }