From f4dc4d584973d3ee7c1369757d4f5900ef9ca061 Mon Sep 17 00:00:00 2001 From: Vismay Golwala Date: Tue, 9 Apr 2019 17:12:42 -0400 Subject: [PATCH] Cap page_size in pagination urls Currently, even with a `max_page_size` of n, we can see urls formed in pagination with `page_size` > n. API still caps the number of results it returns, but the URL remain invalid there. This is a bit messy solution to make string replacement in URL if the query param exceeds `max_page_size` Signed-off-by: Vismay Golwala --- awx/api/pagination.py | 16 ++++++++++++-- .../tests/functional/api/test_pagination.py | 21 +++++++++++++++++++ 2 files changed, 35 insertions(+), 2 deletions(-) diff --git a/awx/api/pagination.py b/awx/api/pagination.py index 9a416e9995..4e4d69c998 100644 --- a/awx/api/pagination.py +++ b/awx/api/pagination.py @@ -18,7 +18,7 @@ class Pagination(pagination.PageNumberPagination): url = self.request and self.request.get_full_path() or '' url = url.encode('utf-8') page_number = self.page.next_page_number() - return replace_query_param(url, self.page_query_param, page_number) + return replace_query_param(self.cap_page_size(url), self.page_query_param, page_number) def get_previous_link(self): if not self.page.has_previous(): @@ -26,4 +26,16 @@ class Pagination(pagination.PageNumberPagination): url = self.request and self.request.get_full_path() or '' url = url.encode('utf-8') page_number = self.page.previous_page_number() - return replace_query_param(url, self.page_query_param, page_number) + return replace_query_param(self.cap_page_size(url), self.page_query_param, page_number) + + def cap_page_size(self, url): + if int(self.request.query_params.get(self.page_size_query_param, 0)) > self.max_page_size: + url = replace_query_param(url, self.page_size_query_param, self.max_page_size) + return url + + def get_html_context(self): + context = super().get_html_context() + context['page_links'] = [pl._replace(url=self.cap_page_size(pl.url)) + for pl in context['page_links']] + + return context diff --git a/awx/main/tests/functional/api/test_pagination.py b/awx/main/tests/functional/api/test_pagination.py index aed0f2e034..528a01cb4b 100644 --- a/awx/main/tests/functional/api/test_pagination.py +++ b/awx/main/tests/functional/api/test_pagination.py @@ -1,7 +1,11 @@ import pytest +import json +from unittest.mock import patch +from urllib.parse import urlencode from awx.main.models.inventory import Group, Host from awx.api.pagination import Pagination +from awx.api.versioning import reverse @pytest.fixture @@ -38,3 +42,20 @@ def test_pagination_backend_output_correct_total_count(group, host): p = Pagination().django_paginator_class(queryset, 10) p.page(1) assert p.count == 1 + + +@pytest.mark.django_db +def test_pagination_cap_page_size(get, admin, inventory): + for i in range(20): + Host(name='host-{}'.format(i), inventory=inventory).save() + + def host_list_url(params): + request_qs = '?' + urlencode(params) + return reverse('api:host_list', kwargs={'version': 'v2'}) + request_qs + + with patch('awx.api.pagination.Pagination.max_page_size', 5): + resp = get(host_list_url({'page': '2', 'page_size': '10'}), user=admin) + jdata = json.loads(resp.content) + + assert jdata['previous'] == host_list_url({'page': '1', 'page_size': '5'}) + assert jdata['next'] == host_list_url({'page': '3', 'page_size': '5'})