From 9a5373d41c4d7f2c295edb381562c1011ed52e12 Mon Sep 17 00:00:00 2001 From: homm Date: Fri, 12 Jun 2015 18:23:12 +0300 Subject: [PATCH 1/7] make PageSizePaginationMixin from all this page_size stuff --- rest_framework/pagination.py | 53 +++++++++++++++++++----------------- 1 file changed, 28 insertions(+), 25 deletions(-) diff --git a/rest_framework/pagination.py b/rest_framework/pagination.py index 5938063af..2b4a14a2d 100644 --- a/rest_framework/pagination.py +++ b/rest_framework/pagination.py @@ -201,7 +201,34 @@ class BasePagination(object): raise NotImplementedError('to_html() must be implemented to display page controls.') -class PageNumberPagination(BasePagination): +class BasePageSizePagination(BasePagination): + # The default page size. + # Defaults to `None`, meaning pagination is disabled. + page_size = api_settings.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. + max_page_size = None + + # Client can control the page size using this query parameter. + # Default is 'None'. Set to eg 'page_size' to enable usage. + page_size_query_param = None + + def get_page_size(self, request): + if self.page_size_query_param: + try: + return _positive_int( + request.query_params[self.page_size_query_param], + strict=True, + cutoff=self.max_page_size + ) + except (KeyError, ValueError): + pass + + return self.page_size + + +class PageNumberPagination(BasePageSizePagination): """ A simple page number based style that supports page numbers as query parameters. For example: @@ -209,21 +236,10 @@ 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 # Client can control the page using this query parameter. page_query_param = 'page' - # Client can control the page size using this query parameter. - # Default is 'None'. Set to eg 'page_size' to enable usage. - page_size_query_param = None - - # 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' @@ -318,19 +334,6 @@ class PageNumberPagination(BasePagination): ('results', data) ])) - def get_page_size(self, request): - if self.page_size_query_param: - try: - return _positive_int( - request.query_params[self.page_size_query_param], - strict=True, - cutoff=self.max_page_size - ) - except (KeyError, ValueError): - pass - - return self.page_size - def get_next_link(self): if not self.page.has_next(): return None From 30b36a594f3f262ca5fcab49ac03d29cab9f2f09 Mon Sep 17 00:00:00 2001 From: homm Date: Fri, 12 Jun 2015 18:57:46 +0300 Subject: [PATCH 2/7] use PageSizePaginationMixin for CursorPagination tests for custom page_size in CursorPagination --- rest_framework/pagination.py | 6 +++--- tests/test_pagination.py | 20 ++++++++++++++++++++ 2 files changed, 23 insertions(+), 3 deletions(-) diff --git a/rest_framework/pagination.py b/rest_framework/pagination.py index 2b4a14a2d..5d0dc70d7 100644 --- a/rest_framework/pagination.py +++ b/rest_framework/pagination.py @@ -487,20 +487,20 @@ class LimitOffsetPagination(BasePagination): return template.render(context) -class CursorPagination(BasePagination): +class CursorPagination(BasePageSizePagination): """ The cursor pagination implementation is neccessarily complex. For an overview of the position/offset style we use, see this post: http://cramer.io/2011/03/08/building-cursors-for-the-disqus-api/ """ cursor_query_param = 'cursor' - page_size = api_settings.PAGE_SIZE invalid_cursor_message = _('Invalid cursor') ordering = '-created' template = 'rest_framework/pagination/previous_and_next.html' def paginate_queryset(self, queryset, request, view=None): - if self.page_size is None: + self.page_size = self.get_page_size(request) + if not self.page_size: return None self.base_url = request.build_absolute_uri() diff --git a/tests/test_pagination.py b/tests/test_pagination.py index c67550c85..1c23c91bb 100644 --- a/tests/test_pagination.py +++ b/tests/test_pagination.py @@ -508,6 +508,8 @@ class TestCursorPagination: class ExamplePagination(pagination.CursorPagination): page_size = 5 + page_size_query_param = 'page_size' + max_page_size = 20 ordering = 'created' self.pagination = ExamplePagination() @@ -643,6 +645,24 @@ class TestCursorPagination: assert isinstance(self.pagination.to_html(), type('')) + def test_page_size(self): + (previous, current, next, previous_url, next_url) = \ + self.get_pages('/?page_size=10') + + assert previous is None + assert current == [1, 1, 1, 1, 1, 1, 2, 3, 4, 4] + assert next == [4, 4, 5, 6, 7, 7, 7, 7, 7, 7] + assert 'page_size=10' in next_url + + (previous, current, next, previous_url, next_url) = \ + self.get_pages(next_url.replace('page_size=10', 'page_size=4')) + + assert previous == [2, 3, 4, 4] + assert current == [4, 4, 5, 6] + assert next == [7, 7, 7, 7] + assert 'page_size=4' in previous_url + assert 'page_size=4' in next_url + def test_get_displayed_page_numbers(): """ From 59905e933541d182617a84545280fbf2890cf5b8 Mon Sep 17 00:00:00 2001 From: homm Date: Mon, 13 Jul 2015 14:36:47 +0300 Subject: [PATCH 3/7] revert BasePageSizePagination class and CursorPagination inheritance --- rest_framework/pagination.py | 59 +++++++++++++++++------------------- 1 file changed, 28 insertions(+), 31 deletions(-) diff --git a/rest_framework/pagination.py b/rest_framework/pagination.py index 5d0dc70d7..5938063af 100644 --- a/rest_framework/pagination.py +++ b/rest_framework/pagination.py @@ -201,34 +201,7 @@ class BasePagination(object): raise NotImplementedError('to_html() must be implemented to display page controls.') -class BasePageSizePagination(BasePagination): - # The default page size. - # Defaults to `None`, meaning pagination is disabled. - page_size = api_settings.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. - max_page_size = None - - # Client can control the page size using this query parameter. - # Default is 'None'. Set to eg 'page_size' to enable usage. - page_size_query_param = None - - def get_page_size(self, request): - if self.page_size_query_param: - try: - return _positive_int( - request.query_params[self.page_size_query_param], - strict=True, - cutoff=self.max_page_size - ) - except (KeyError, ValueError): - pass - - return self.page_size - - -class PageNumberPagination(BasePageSizePagination): +class PageNumberPagination(BasePagination): """ A simple page number based style that supports page numbers as query parameters. For example: @@ -236,10 +209,21 @@ class PageNumberPagination(BasePageSizePagination): 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 # Client can control the page using this query parameter. page_query_param = 'page' + # Client can control the page size using this query parameter. + # Default is 'None'. Set to eg 'page_size' to enable usage. + page_size_query_param = None + + # 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' @@ -334,6 +318,19 @@ class PageNumberPagination(BasePageSizePagination): ('results', data) ])) + def get_page_size(self, request): + if self.page_size_query_param: + try: + return _positive_int( + request.query_params[self.page_size_query_param], + strict=True, + cutoff=self.max_page_size + ) + except (KeyError, ValueError): + pass + + return self.page_size + def get_next_link(self): if not self.page.has_next(): return None @@ -487,20 +484,20 @@ class LimitOffsetPagination(BasePagination): return template.render(context) -class CursorPagination(BasePageSizePagination): +class CursorPagination(BasePagination): """ The cursor pagination implementation is neccessarily complex. For an overview of the position/offset style we use, see this post: http://cramer.io/2011/03/08/building-cursors-for-the-disqus-api/ """ cursor_query_param = 'cursor' + page_size = api_settings.PAGE_SIZE invalid_cursor_message = _('Invalid cursor') ordering = '-created' template = 'rest_framework/pagination/previous_and_next.html' def paginate_queryset(self, queryset, request, view=None): - self.page_size = self.get_page_size(request) - if not self.page_size: + if self.page_size is None: return None self.base_url = request.build_absolute_uri() From a3afcfb6d3ff454860cd1d39082bd1788fd98d58 Mon Sep 17 00:00:00 2001 From: homm Date: Mon, 13 Jul 2015 14:44:48 +0300 Subject: [PATCH 4/7] duplicate `get_page_size` and related properties from PageNumberPagination --- rest_framework/pagination.py | 28 +++++++++++++++++++++++++++- 1 file changed, 27 insertions(+), 1 deletion(-) diff --git a/rest_framework/pagination.py b/rest_framework/pagination.py index 5938063af..f70a6edad 100644 --- a/rest_framework/pagination.py +++ b/rest_framework/pagination.py @@ -491,13 +491,26 @@ class CursorPagination(BasePagination): http://cramer.io/2011/03/08/building-cursors-for-the-disqus-api/ """ cursor_query_param = 'cursor' + + # The default page size. + # Defaults to `None`, meaning pagination is disabled. page_size = api_settings.PAGE_SIZE + + # Client can control the page size using this query parameter. + # Default is 'None'. Set to eg 'page_size' to enable usage. + page_size_query_param = None + + # 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 + invalid_cursor_message = _('Invalid cursor') ordering = '-created' template = 'rest_framework/pagination/previous_and_next.html' def paginate_queryset(self, queryset, request, view=None): - if self.page_size is None: + self.page_size = self.get_page_size(request) + if not self.page_size: return None self.base_url = request.build_absolute_uri() @@ -577,6 +590,19 @@ class CursorPagination(BasePagination): return self.page + def get_page_size(self, request): + if self.page_size_query_param: + try: + return _positive_int( + request.query_params[self.page_size_query_param], + strict=True, + cutoff=self.max_page_size + ) + except (KeyError, ValueError): + pass + + return self.page_size + def get_next_link(self): if not self.has_next: return None From 14a32ae32d6b4605b41b0929bab98ecb9e1f67c0 Mon Sep 17 00:00:00 2001 From: Tom Christie Date: Tue, 14 Jul 2015 11:40:15 +0100 Subject: [PATCH 5/7] Add get_page_size hook to CursorPagination. Closes #3068. --- rest_framework/pagination.py | 18 ------------------ tests/test_pagination.py | 20 -------------------- 2 files changed, 38 deletions(-) diff --git a/rest_framework/pagination.py b/rest_framework/pagination.py index 081760e1c..3536f793a 100644 --- a/rest_framework/pagination.py +++ b/rest_framework/pagination.py @@ -454,14 +454,6 @@ class CursorPagination(BasePagination): # Defaults to `None`, meaning pagination is disabled. page_size = api_settings.PAGE_SIZE - # Client can control the page size using this query parameter. - # Default is 'None'. Set to eg 'page_size' to enable usage. - page_size_query_param = None - - # 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 - invalid_cursor_message = _('Invalid cursor') ordering = '-created' template = 'rest_framework/pagination/previous_and_next.html' @@ -544,16 +536,6 @@ class CursorPagination(BasePagination): return self.page def get_page_size(self, request): - if self.page_size_query_param: - try: - return _positive_int( - request.query_params[self.page_size_query_param], - strict=True, - cutoff=self.max_page_size - ) - except (KeyError, ValueError): - pass - return self.page_size def get_next_link(self): diff --git a/tests/test_pagination.py b/tests/test_pagination.py index 0ab149d07..03c4fdf47 100644 --- a/tests/test_pagination.py +++ b/tests/test_pagination.py @@ -513,8 +513,6 @@ class TestCursorPagination: class ExamplePagination(pagination.CursorPagination): page_size = 5 - page_size_query_param = 'page_size' - max_page_size = 20 ordering = 'created' self.pagination = ExamplePagination() @@ -650,24 +648,6 @@ class TestCursorPagination: assert isinstance(self.pagination.to_html(), type('')) - def test_page_size(self): - (previous, current, next, previous_url, next_url) = \ - self.get_pages('/?page_size=10') - - assert previous is None - assert current == [1, 1, 1, 1, 1, 1, 2, 3, 4, 4] - assert next == [4, 4, 5, 6, 7, 7, 7, 7, 7, 7] - assert 'page_size=10' in next_url - - (previous, current, next, previous_url, next_url) = \ - self.get_pages(next_url.replace('page_size=10', 'page_size=4')) - - assert previous == [2, 3, 4, 4] - assert current == [4, 4, 5, 6] - assert next == [7, 7, 7, 7] - assert 'page_size=4' in previous_url - assert 'page_size=4' in next_url - def test_get_displayed_page_numbers(): """ From 2b51d5594b727e7a14b6e1c3c94e325898fb5213 Mon Sep 17 00:00:00 2001 From: Tom Christie Date: Tue, 14 Jul 2015 11:41:25 +0100 Subject: [PATCH 6/7] Remove whitespace and comment changes --- rest_framework/pagination.py | 4 ---- 1 file changed, 4 deletions(-) diff --git a/rest_framework/pagination.py b/rest_framework/pagination.py index 3536f793a..a171c684f 100644 --- a/rest_framework/pagination.py +++ b/rest_framework/pagination.py @@ -449,11 +449,7 @@ class CursorPagination(BasePagination): http://cramer.io/2011/03/08/building-cursors-for-the-disqus-api/ """ cursor_query_param = 'cursor' - - # The default page size. - # Defaults to `None`, meaning pagination is disabled. page_size = api_settings.PAGE_SIZE - invalid_cursor_message = _('Invalid cursor') ordering = '-created' template = 'rest_framework/pagination/previous_and_next.html' From 468361425d38f27580626f0ae9af74d9c4f7bc98 Mon Sep 17 00:00:00 2001 From: Tom Christie Date: Tue, 14 Jul 2015 12:32:27 +0100 Subject: [PATCH 7/7] page_size should be local, not overwriting class attribute. --- rest_framework/pagination.py | 8 ++++---- 1 file changed, 4 insertions(+), 4 deletions(-) diff --git a/rest_framework/pagination.py b/rest_framework/pagination.py index a171c684f..a14141d3c 100644 --- a/rest_framework/pagination.py +++ b/rest_framework/pagination.py @@ -455,8 +455,8 @@ class CursorPagination(BasePagination): template = 'rest_framework/pagination/previous_and_next.html' def paginate_queryset(self, queryset, request, view=None): - self.page_size = self.get_page_size(request) - if not self.page_size: + page_size = self.get_page_size(request) + if not page_size: return None self.base_url = request.build_absolute_uri() @@ -491,8 +491,8 @@ class CursorPagination(BasePagination): # If we have an offset cursor then offset the entire page by that amount. # We also always fetch an extra item in order to determine if there is a # page following on from this one. - results = list(queryset[offset:offset + self.page_size + 1]) - self.page = list(results[:self.page_size]) + results = list(queryset[offset:offset + page_size + 1]) + self.page = list(results[:page_size]) # Determine the position of the final item following the page. if len(results) > len(self.page):